Support multiple meshes in R2S calculations#3860
Support multiple meshes in R2S calculations#3860paulromano wants to merge 4 commits intoopenmc-dev:developfrom
Conversation
eepeterson
left a comment
There was a problem hiding this comment.
Thanks for the helpful improvement @paulromano. A number of small comments for your consideration, but nothing blocking.
| model : openmc.Model | ||
| OpenMC model object. Must contain geometry, materials, and settings. | ||
| domains : list of openmc.Material or openmc.Cell or openmc.Universe, or openmc.MeshBase, or openmc.Filter | ||
| domains : list of openmc.Material or openmc.Cell or openmc.Universe, or openmc.MeshBase, or openmc.Filter, or list of openmc.Filter |
There was a problem hiding this comment.
This list feels like it's growing a bit long and having the general openmc.Filter and list of openmc.Filter in the documentation may be slightly misleading since not all Filter objects should be viable here.
Makes me wonder if it's worth creating an openmc.Domain enum-like type that does the type checking and validation for us and leveraging that here (and elsewhere potentially). The arguments to domains are defining spatial regions regardless of whether it is implicitly defined through something else like Material assignments. It might be nice to have a type or class that checks that what's passed is a valid way of defining some spatial domain.
It may be obvious what should and should not be allowed if someone is using this function, but it might help to be more explicit.
| domain_filters = [openmc.CellFilter(domains)] | ||
| elif isinstance(domains[0], openmc.Universe): | ||
| domain_filter = openmc.UniverseFilter(domains) | ||
| domain_filters = [openmc.UniverseFilter(domains)] |
There was a problem hiding this comment.
If we do move forward with the above suggested abstraction this code could migrate.
| for flux_arr, rr_tally in zip(all_flux_arrays, rr_tallies): | ||
| flux = flux_arr | ||
| reaction_rates = rr_tally.get_reshaped_data() | ||
| reaction_rates = np.moveaxis(reaction_rates, 1, -1) |
There was a problem hiding this comment.
could be worth adding same comment here as above in the flux loop to make clear what is being aligned.
| ---------- | ||
| domains : openmc.MeshBase or Sequence[openmc.Cell] | ||
| The mesh or a sequence of cells that represent the spatial units over | ||
| domains : list of openmc.MeshBase or Sequence[openmc.Cell] |
There was a problem hiding this comment.
Actually seeing here that we use domains for a few different things, that may negate my previous comment, but I'll leave it for posterity.
| if mmv_file.exists(): | ||
| self.results['mesh_material_volumes'] = \ | ||
| openmc.MeshMaterialVolumes.from_npz(mmv_file) | ||
| mmv_files = sorted(neutron_dir.glob('mesh_material_volumes*.npz'), |
There was a problem hiding this comment.
might be worth including digit specifiers in the glob string to ensure what you're getting is indeed what you intend to sort. Same goes for below in the photon glob search.
Description
This PR extends
R2SManagerto accept a list of meshes (as thedomainsargument), enabling mesh-based R2S calculations to use multiple non-overlapping meshes in a single workflow. Each element-material combination across all meshes is treated as an independent activation region. This can come in handy in a variety of situations:Checklist