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

Consolidate plotting capabilities in Model.plot #3282

Merged
merged 8 commits into from
Jan 31, 2025

Conversation

paulromano
Copy link
Contributor

Description

This PR is mostly a refactor of the following plot methods:

  • Model.plot
  • Geometry.plot
  • Universe.plot
  • Cell.plot
  • Region.plot

Right now, most of the actual plotting logic is in Universe.plot and the other methods just call that. With the changes here, the logic has been moved to Model.plot, which will enable plotting to work for DAGMC geometries (which require model.materials). I've made a few other improvements and fixes, including:

  • The plane_tolerance argument would not accept an int due to a type check; this has been fixed
  • The outline argument now accepts either a bool or the string "only", which will only show the outline.
  • I've added a contour_kwargs argument that enables customization of the lines shown when outline is True.
  • A @add_plot_params decorator is used to list the parameters for the docstrings consistently, which means if we add or change an argument, you only have to update one string instead of every docstring for the classes above.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@paulromano paulromano requested a review from shimwell January 29, 2025 14:19
openmc/universe.py Outdated Show resolved Hide resolved
Copy link
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

Looks really nice @paulromano. Thanks for cleaning this up!

I'll allow a little bit of a cooldown -- merging tomorrow.

@pshriwise pshriwise added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Jan 30, 2025
@shimwell
Copy link
Member

I did a little plotting with this and didn't see any errors so far.

@pshriwise pshriwise merged commit 59c398b into openmc-dev:develop Jan 31, 2025
16 checks passed
@paulromano paulromano deleted the plotting-improvements branch February 2, 2025 01:39
ahnaf-tahmid-chowdhury pushed a commit to ahnaf-tahmid-chowdhury/OpenMC that referenced this pull request Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merging Soon PR will be merged in < 24 hrs if no further comments are made.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants