feature/split plot classes #3528
Conversation
bpolania
commented
Aug 11, 2025
bpolania
commented
- Add stat:sum field to MCPL files for proper weight normalization (MCPL integration in OpenMC #3514)
- feat: split Plot class into SlicePlot and VoxelPlot
- fix: enforce strict width/pixels validation for plot classes
- test: update tests to use new SlicePlot and VoxelPlot classes
- test: add comprehensive tests for SlicePlot and VoxelPlot classes
…nmc-dev#3514) - Implements stat:sum field (key: "openmc_np1") in MCPL file headers - Initially sets to -1 for crash safety, updates with particle count before closing - Compatible with MCPL >= 2.1.0, gracefully degrades for older versions - Enables proper file merging and McStas/McXtrace integration - Adds C++ and Python unit tests
This refactoring improves the plot interface by: - Creating dedicated SlicePlot class for 2D slice plots - Creating dedicated VoxelPlot class for 3D voxel plots - Maintaining backward compatibility with deprecated Plot class - Properly separating attributes specific to each plot type The width attribute now accepts 2 values for slice plots and 3 for voxel plots. The basis attribute only exists on SlicePlot as it doesn't apply to voxel plots. Addresses openmc-dev#3507
- SlicePlot now strictly accepts 2 values for width and pixels - VoxelPlot now strictly accepts 3 values for width and pixels - Fixed check_length calls to use both min and max parameters - Ensures proper separation between 2D and 3D plot types
- Updated test_voxel_plot to use VoxelPlot directly - Updated test_plot_directory to use SlicePlot - Updated test_highlight_domains to use SlicePlot - Updated test_plots to use both SlicePlot and VoxelPlot - Fixed VoxelPlot default pixels to be [400, 400, 400] - Kept some tests using legacy Plot class for backward compatibility testing
- Test initialization with proper defaults - Test width/pixels validation (2D vs 3D) - Test basis attribute presence/absence - Test meshlines attribute presence/absence - Test XML serialization/deserialization - Test backward compatibility with Plot class - Test deprecation warnings - Test Plots collection with mixed types - Follow OpenMC testing conventions
Resolved conflicts in: - openmc/plots.py: Updated to use get_text() pattern from upstream - src/mcpl_interface.cpp: Merged both hdr_add_data and hdr_add_stat_sum additions - tests/unit_tests/test_mcpl_stat_sum.py: Used upstream version
…s method The Plot.from_geometry() class method now suppresses the deprecation warning internally since it's a legitimate use case for backward compatibility.
Remove incorrect line break in mcpl_read_fpt type definition that was introduced during merge conflict resolution.
|
I like this PR and would be keen to see it merged. Originally I noticed +903 lines and thought that was a lot but +327 lines are tests 👍 which we didn't have before and another ~300 lines are the Legacy Plot class which would presumably be deleted after a few releases. So I think this is a nice approach to split the 3D voxel and 2D slice plot out. This PR would close issue #3507 |
|
Sorry to ask I know everyone is super busy. I am keen to make a small change to the plotting in openmc to mirror a recent feature added to the plotter. I've been using it in the plotter and could do with the same feature in the Python API. It would be useful to know if this PR is getting merged or not as I hope to make changes to the files changed by this PR. I'm trying to avoid generating conflicts with the changes here. I'm still very much in support of this PR by want to give others a chance to review before merging |
|
Keen to know if this PR is ok to merge as I noticed an issue in the geometry plot function that I'm keen to fix but I am keen to avoid causing conflicts for this PR |
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
I'm also keen on this PR as well -- I'll try to review soon but putting a placeholder for now.
@shimwell The outline feature is already available in our plot methods already (outline argument).
|
This other issue is unrelated to the outline but I would like to fix it so I can plot geometry with the source and with axis units in meters which currently has a bug |
|
ax = model.plot(color_by='material', ...)
model.plot(color_by='cell', outline='only', axes=ax, ...) |
I don't want to go to far of the topic from this PR which is splitting the plot into slice and voxel plots. I think the above code would generate two plots and combine them, taking twice as long to plot but yes that would work. The option that I think does not exist in the python API that I would like to add without causing conflicts wit this PR is |
3 participants
