Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #3158 - Streamline use of CompositeSurface with SurfaceFilter #3167

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
6 changes: 6 additions & 0 deletions openmc/model/surface_composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ def rotate(self, rotation, pivot=(0., 0., 0.), order='xyz', inplace=False):
s = getattr(surf, name)
setattr(surf, name, s.rotate(rotation, pivot, order, inplace))
return surf

def get_surfaces(self):
zoeprieto marked this conversation as resolved.
Show resolved Hide resolved
return [getattr(self, name) for name in self._surface_names]

def get_id_surfaces(self):
return [getattr(self, name).id for name in self._surface_names]
zoeprieto marked this conversation as resolved.
Show resolved Hide resolved

@property
def boundary_type(self):
Expand Down
40 changes: 40 additions & 0 deletions tests/unit_tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,3 +294,43 @@ def test_tabular_from_energyfilter():

tab = efilter.get_tabular(values=np.array([10, 10, 5]), interpolation='linear-linear')
assert tab.interpolation == 'linear-linear'


def test_SurfaceFilter_CompositeSurface(run_in_tmpdir, box_model):
m = openmc.Material()
m.add_nuclide('U235', 1.0)
m.set_density('g/cm3', 1.0)

box = openmc.model.RectangularPrism(10., 10., boundary_type='vacuum')
c = openmc.Cell(fill=m, region=-box)
box_model.geometry.root_universe = openmc.Universe(cells=[c])

tally = openmc.Tally()
tally.filters = [openmc.SurfaceFilter(box.get_surfaces())]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think better yet we'd be able to pass a CompositeSurface directly to the SurfaceFilter class, which might require some additional handling in that object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it is clearer. It is already done. I thought it was easier to change the SurfaceFilter class than to make a CompositeSurface always return a list of surfaces. Let me know what you think.

tally.scores = ['current']

box_model.tallies = [tally]

sp_name = box_model.run()

with openmc.StatePoint(sp_name) as sp:
current = sp.tallies[tally.id]
assert len(current.get_pandas_dataframe()['surface']) == 4
assert np.all(current.get_pandas_dataframe()['surface'] == box.get_id_surfaces())

box = openmc.model.RectangularParallelepiped(*[-10, 10]*3, boundary_type='vacuum')
c = openmc.Cell(fill=m, region=-box)
box_model.geometry.root_universe = openmc.Universe(cells=[c])

tally = openmc.Tally()
tally.filters = [openmc.SurfaceFilter(box.get_surfaces())]
tally.scores = ['current']

box_model.tallies = [tally]

sp_name = box_model.run()

with openmc.StatePoint(sp_name) as sp:
current = sp.tallies[tally.id]
assert len(current.get_pandas_dataframe()['surface']) == 6
assert np.all(current.get_pandas_dataframe()['surface'] == box.get_id_surfaces())