Skip to content

Comments

Introduce new C API function for slice plots#3806

Open
paulromano wants to merge 14 commits intoopenmc-dev:developfrom
paulromano:slice-plot-api
Open

Introduce new C API function for slice plots#3806
paulromano wants to merge 14 commits intoopenmc-dev:developfrom
paulromano:slice-plot-api

Conversation

@paulromano
Copy link
Contributor

Description

This PR introduces a new openmc_slice_plot 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_plot 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_plot 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_plot 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_plot 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_plot subsumes their capability.
  2. How should this be presented on the Model class? Currently I have a separate slice_plot 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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant