Introduce new C API function for slice plots

[paulromano]

Description

This PR introduces a new openmc_slice_data C API function that addresses several limitations of the existing API:

• Performance: When the openmc-plotter application makes calls via the C API, is has to call both openmc_id_map (for cell/material IDs) and openmc_property_map (for densities/temperatures). Both of these calls involve the same loop over pixels with a "find cell" call at each pixel. In the openmc_slice_data function, both IDs and properties are retrieved at the same time, which will reduce the time for each slice plot in the plotter by 2×.
• Enhanced tally visualization: The plotter currently relies on ad hoc logic to perform visualization of spatial tally data. Matching each pixel to a matching filter bin could be done much more intuitively on the C++ side. My first shot at this was to have an openmc_filter_get_plot_bins function that is similar to the existing openmc_mesh_get_plot_bins, but once again this would involve another loop over pixels with "find cell" calls (because filters rely on a properly constructed particle with geometry state information). Instead, I've designed openmc_slice_data to optionally take a filter index and return matching bin indices for each pixel. This will both speed up tally visualization in the plotter and allow us to plot things we can't currently (like MeshMaterialFilter).
• Arbitrary orientation slice plots: Our current slice plotting capability only handles axis-aligned slices. openmc_slice_data is more general and allows arbitrary orientation slice plots by specifying two orthogonal span vectors that define the view.
• Cleaner interface: The openmc_id_map and openmc_property_map calls rely on a struct that has to be created on the Python side matching the C struct internally, which has always felt awkward to me. openmc_slice_data is designed to take plain basic datatypes to avoid this.

Design questions

1. If we're OK moving forward with this, it begs the question of whether we should keep id_map, property_map, and _PlotBase around since slice_data subsumes their capability.
2. How should this be presented on the Model class? Currently I have a separate slice_data method but arguably it can (should?) be integrated into the existing plot method?

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]

If any of you want to try this out in the plotter, I've just put up a draft PR there: openmc-dev/plotter#172.

[pshriwise]

I think this looks great @paulromano. It should save us a lot of time plotting, particularly for models where find_cell is expensive. I know there's been a lot of desire for off-axis plots as well. Relatively minor comments from me here, but more to say on design and some of the questions you raised.

One limitation I see here that might be problematic is the specification of a single Filter. It would require a fair amount to add support for more than one I know, but if we want to rely on the filter matching in a more general way for plotting then I think we may need to support multiple Filters. (e.g. I often see mesh and cell/material filters used on the same tally).

For the plotter utility specifically it might be additionally beneficial to request the filter matches for all spatial filters at once so that the filter bin map doesn't need to be re-generated when switching tallies if the view itself doesn't change.

As for the id_map and property_map methods, I don't see any reason why we wouldn't deprecate them and have them return the appropriate subset of data from slice_plot for now.

Regarding Model.plot, Model.slice_plot returns raw data about the model (wondering if we might rename that to Model.slice in fact) and Model.plot does a lot of extra work to display an image, so we can keep the plotting bells & whistles there and it can rely on Model.slice_plot for the image data.

[paulromano]

@pshriwise Thanks for the review! To respond to some of your comments above:

One limitation I see here that might be problematic is the specification of a single Filter. It would require a fair amount to add support for more than one I know, but if we want to rely on the filter matching in a more general way for plotting then I think we may need to support multiple Filters. (e.g. I often see mesh and cell/material filters used on the same tally).

For the plotter utility specifically it might be additionally beneficial to request the filter matches for all spatial filters at once so that the filter bin map doesn't need to be re-generated when switching tallies if the view itself doesn't change.

With respect to the plotter, we only use one filter to produce the resulting image so I don't see how this would help us much. There's nowhere to "put" the second filter's spatial information in the image. And as far as not needing to re-generate information when switching tallies, we already have special purpose logic for the cell/material case since we have their ID data directly, so those wouldn't require any re-generation. So overall, I don't see having the ability to handle multiple filters providing much value and simply adding more complexity to the interfaces.

As for the id_map and property_map methods, I don't see any reason why we wouldn't deprecate them and have them return the appropriate subset of data from slice_plot for now.

I've gone ahead with deprecation of these and also updated to always delegate to slice_plot where possible.

Regarding Model.plot, Model.slice_plot returns raw data about the model (wondering if we might rename that to Model.slice in fact) and Model.plot does a lot of extra work to display an image, so we can keep the plotting bells & whistles there and it can rely on Model.slice_plot for the image data.

Yeah you're right that these wouldn't make sense to combine. slice as a name seems a little vague and could be misinterpreted; the primary purpose is for slice plotting so my preference is to keep it as slice_plot.

---

Let me know if you have any further requests!

[pshriwise]

Yeah you're right that these wouldn't make sense to combine. slice as a name seems a little vague and could be misinterpreted; the primary purpose is for slice plotting so my preference is to keep it as slice_plot.

One qualm I have about slice_plot is that we're generating that's intended for use in plotting, but we're not really plotting anything per se. Agreed that slice is too vague though. Does the method name slice_data resonate?

[paulromano]

@pshriwise Yes, that works for me. I've gone ahead and renamed to slice_data. Thanks for the suggestion!

[pshriwise]

Looking at this again today and had a thought -- now that we're passing a Particle instead of a GeometryState would it make sense to use the filter_matches_ vector on the particle itself instead of passing a pointer here?

[pshriwise]

Ah, our particle is passed as const. I see. I'll leave this for a refactor in the future perhaps. My brain is a bit stuck on it so I might give that a try in a follow-on mini-PR. No reason to hold this up for now imo.

[paulromano]

Yeah, either way would work (if the const were removed). I chose to have FilterMatch separate because the filter_matches_ member is currently utilized to preserve the filter matching state during actual tallying, and looking up a match for plotting purposes felt orthogonal to that. Other than removing the FilterMatch* argument getting passed, it wouldn't change much else.

In any case, thanks for the thoughtful review!

[pshriwise]

Thank you @paulromano! Excited to see these improvements reflected in the plotter!