Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closing all the plots that ARMI opens #1698

Merged
merged 8 commits into from May 16, 2024
Merged

Closing all the plots that ARMI opens #1698

merged 8 commits into from May 16, 2024

Conversation

john-science
Copy link
Member

What is the change?

This PR makes sure that matplotlib plots in ARMI are closed.

Why is the change being made?

Someone recently complained to me that during their recent ARMI Application runs, they were getting matplotlib warnings that they had more than 20 plots open at a time.

I believe this is the result of an old design idea that ARMI should traffic in figure objects, passing them around freely. Maybe it was a nice idea, we could discuss, but I think it leaves open the real possibility that plots aren't closed when they should be.


Checklist

  • The release notes have been updated if necessary.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in pyproject.toml.

@john-science john-science added the cleanup Code/comment cleanup: Low Priority label May 3, 2024
@john-science
Copy link
Member Author

I think a lot of ARMI PRs are failing right now because the unit tests aren't cleaning up after themselves.

In particular, after all the tests are run, a file named armiRun.h5 appears to be left in the base dir of the working area.

This should be remedied.

@opotowsky
Copy link
Member

I think a lot of ARMI PRs are failing right now because the unit tests aren't cleaning up after themselves.

In particular, after all the tests are run, a file named armiRun.h5 appears to be left in the base dir of the working area.

This should be remedied.

Interesting that it looks like it's only happening on windows and not linux

@john-science
Copy link
Member Author

So, matplotlib released a new version to PyPI.org today. This removed (or changed?) the old color map logic we are using:

colormap = cm.get_cmap("jet")

You will see get_cmap() in matplotlib==3.7.5, but not in matplotlib==3.8.4.

@john-science john-science merged commit ccefa38 into main May 16, 2024
21 checks passed
@john-science john-science deleted the close_your_plots branch May 16, 2024 16:44
@@ -404,13 +404,12 @@ def plotFaceMap(
)
elif referencesToKeep:
# Don't show yet, since it will be updated.
plt.close(fig)
Copy link
Contributor

@onufer onufer May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please Delete this one. it cant close with plotBlockDepthMap

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ope! @john-science I guess we didn't test this internally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code/comment cleanup: Low Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants