openmc-dev / openmc

OpenMC Monte Carlo Code
https://docs.openmc.org
Other
699 stars 444 forks source link

Create convenience option for get_all_surfaces to filter by surface type #2924

Open MicahGale opened 1 month ago

MicahGale commented 1 month ago

Description

Recently I was trying to find the bounding planes for a problem (i.e., the openmc.Zplane with the max and min values, or boundary_type == 'vacuum'). After some discussion with @paulromano I think the idiomatic way would be to update openmc.Region.get_surfaces and openmc.Geometry.get_all_surfaces to have a signature more like:

Note: type annotations just added for clarity.

def get_all_surfaces(self, 
                    surface_type_filter: Union(Class, str) = None, 
                    boundary_type_filter: str = None
) -> Dict[int, openmc.Surface]:
   pass

Notes:

  1. enforcement for valid filters will need to fail fast.
  2. The stringified versions of the class names should be pythonic. The question is should be they be treated as classes or instances? i.e., 'z_plane' vs. 'ZPlane'.
  3. It may or may not be worthwhile to add an SurfaceType enum.
  4. AFAIK openmc.Universe does not have a get_all_surfaces
  5. None options for the filter default to no filtering at all.

Tasks:

  1. [ ] Add filter by surface type
  2. [ ] Add filter by boundary type
  3. [ ] Add universe.get_all_surfaces

Alternatives

The currently work around is for the user to do this manually. This method given would take O(2n) versus a possible O(n) implementation. Obviously these are the same order of scaling but still not ideal:

actual_surfaces = {}
for id, surf in model.geometry.get_all_surfaces().items():
   if isinstance(surf, openmc.ZPlane):
      actual_surfaces[id] = surf

Compatibility

This should not break backwards compatibility as all arguments will be optional and without them the status quo is maintained.

I can tackle this ... at some point. Probably after this spring semester.