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

Cylindrical prism with optional filleted edges #2309

Merged
merged 26 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
033ecb0
add cylindrical_prism function
yardasol Nov 22, 2022
55abbd3
Add unit test for cylindrical_prism
yardasol Nov 23, 2022
2409d0e
Merge branch 'develop' into rounded-cyl-corners
yardasol Nov 23, 2022
69d2bb5
Apply @paulromano 's suggestions from code review
yardasol Dec 8, 2022
f0d55e3
add fillet terminology
yardasol Dec 8, 2022
56be597
Merge remote-tracking branch 'origin/rounded-cyl-corners' into rounde…
yardasol Dec 8, 2022
0236e4b
Apply @pshriwise suggestions from code review
yardasol Dec 8, 2022
f77f6c1
pep8 fixes
yardasol Dec 8, 2022
e3c573b
fix return string
yardasol Dec 8, 2022
2fa0c96
rewrite test_cyl_prism()
yardasol Dec 8, 2022
974c706
add helper function to reduce code reuse
yardasol Dec 8, 2022
6ca6f4d
Add cylindrical_prism() to the API docs
yardasol Dec 8, 2022
e1d27a9
Use @gonuke's suggestion for simplifying create_fillet()
yardasol Dec 9, 2022
c767ea3
make use of getattr consistent; use shared _plane function
yardasol Dec 9, 2022
3e24dcd
fillet_upper -> upper_fillet; fillet_lower -> lower_fillet
yardasol Dec 9, 2022
8dc3983
cleanup create_fillet()
yardasol Dec 9, 2022
052b5c2
Add @pshriwise's input for the unit test
yardasol Dec 9, 2022
24432ea
fix unit test
yardasol Dec 9, 2022
11ab97f
simplify import statements
yardasol Dec 9, 2022
84d35a7
move fillet functionality to RightCircularCylinder
yardasol Feb 24, 2023
896bb1c
add additional surfaces to docstring
yardasol Jun 11, 2023
26a21eb
Merge branch 'develop' into rounded-cyl-corners
yardasol Jun 11, 2023
c28ef4a
cleanup
yardasol Jun 13, 2023
a1f3071
typo fix
yardasol Jun 14, 2023
2023006
Merge remote-tracking branch 'upstream/develop' into rounded-cyl-corners
yardasol Jun 26, 2023
e9cb99c
Apply suggestions from @paulromano code review
yardasol Jul 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 137 additions & 2 deletions openmc/model/funcs.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
from warnings import warn

from openmc import (
XPlane, YPlane, Plane, ZCylinder, Cylinder, XCylinder,
YCylinder, Universe, Cell)
XPlane, YPlane, ZPlane, Plane, ZCylinder, Cylinder, XCylinder,
YCylinder, ZTorus, XTorus, YTorus,
Universe, Cell)
from ..checkvalue import (
check_type, check_value, check_length, check_less_than,
check_iterable_type)
Expand Down Expand Up @@ -110,6 +111,140 @@ def borated_water(boron_ppm, temperature=293., pressure=0.1013, temp_unit='K',
out.add_s_alpha_beta('c_H_in_H2O')
return out

def cylindrical_prism(r, height, axis='z', origin=(0., 0., 0.),
yardasol marked this conversation as resolved.
Show resolved Hide resolved
boundary_type='transmission',
upper_edge_radius=0.,
lower_edge_radius=0.):
"""Get a finite cylindrical prism.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps more information here about the rounded edges. From reviewing the code, it seems that this option adds what I would call a fillet at each end.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for fillet terminology


Parameters
----------
r: float
Prism radius in units of cm.
height: float
Prism height in units of cm. The height is aligned with the x, y,
or z axes for prisms parallel to the x, y, or z axis, respectively.
axis : {'x', 'y', 'z'}
Axis with which the infinite length of the prism should be aligned.
Defaults to 'z'.
origin: Iterable of three floats
Origin of the prism. The three floats correspond to (x,y,z)
Defaults to (0., 0., 0.).
boundary_type : {'transmission, 'vacuum', 'reflective', 'periodic'}
Boundary condition that defines the behavior for particles hitting the
surfaces comprising the rectangular prism (default is 'transmission').
upper_edge_radius: float
Prism upper edge radius in units of cm. Defaults to 0.
lower_edge_radius: float
Prism lower edge radius in units of cm. Defaults to 0.
yardasol marked this conversation as resolved.
Show resolved Hide resolved

Returns
-------
openmc.Region
The inside of a rectangular prism
yardasol marked this conversation as resolved.
Show resolved Hide resolved

"""


check_type('r', r, Real)
check_type('height', height, Real)
check_type('upper_edge_radius', upper_edge_radius, Real)
check_less_than('upper_edge_radius', upper_edge_radius, r, equality=True)
check_type('lower_edge_radius', lower_edge_radius, Real)
check_less_than('lower_edge_radius', lower_edge_radius, r, equality=True)
check_value('axis', axis, ['x', 'y', 'z'])
check_type('origin', origin, Iterable, Real)

# Define function to create a plane on given axis
def plane(axis, name, value):
cls = globals()['{}Plane'.format(axis.upper())]
return cls(name='{} {}'.format(name, axis),
boundary_type=boundary_type,
**{axis + '0': value})
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of globals() feels a little hacky. I'd rather we import openmc and then get the object from that. Also, if you put the value of the axis first when creating the plane, it will simplify it a bit:

Suggested change
cls = globals()['{}Plane'.format(axis.upper())]
return cls(name='{} {}'.format(name, axis),
boundary_type=boundary_type,
**{axis + '0': value})
cls = getattr(openmc, f'{axis.upper()}Plane')
return cls(value, name=f'{name} {axis}',
boundary_type=boundary_type)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The globals() approach is used in rectangular_prism and hexagonal_prism so these should probably get changed as well.

def cylinder(axis, name, ax1, val1, ax2, val2, r):
cls = globals()['{}Cylinder'.format(axis.upper())]
return cls(name='{} {}'.format(name, axis),
yardasol marked this conversation as resolved.
Show resolved Hide resolved
boundary_type=boundary_type,
**{ax1 + '0': val1,
ax2 + '0': val2,
'r': r})

if axis == 'x':
x1, x2 = 'y', 'z'
axcoord = 0
axcoord1 = 1
axcoord2 = 2
elif axis == 'y':
x1, x2 = 'x', 'z'
axcoord = 1
axcoord1 = 0
axcoord2 = 2
else:
x1, x2 = 'x', 'y'
axcoord = 2
axcoord1 = 0
axcoord2 = 1

# Get torus class corresponding to given axis
tor = globals()['{}Torus'.format(axis.upper())]
yardasol marked this conversation as resolved.
Show resolved Hide resolved

# Create cylindricalr region
yardasol marked this conversation as resolved.
Show resolved Hide resolved
min_h = plane(axis, 'minimum', -height/2 + origin[axcoord])
max_h = plane(axis, 'maximum', height/2 + origin[axcoord])
radial = cylinder(axis, 'outer', x1, origin[axcoord1], x2, origin[axcoord2], r)

if boundary_type == 'periodic':
min_h.periodic_surface = max_h
max_h.periodic_surface = min_h
prism = +min_h & -max_h & -radial

# Handle rounded corners if given
if upper_edge_radius > 0. or lower_edge_radius > 0.:
if boundary_type == 'periodic':
raise ValueError('Periodic boundary conditions not permitted when '
'rounded corners are used.')

corners_upper = None
corners_lower = None
args = {'boundary_type': boundary_type}
args[x1 + '0'] = origin[axcoord1]
args[x2 + '0'] = origin[axcoord2]
yardasol marked this conversation as resolved.
Show resolved Hide resolved
if upper_edge_radius > 0.:
args['a'] = r - upper_edge_radius
args['b'] = upper_edge_radius
args['c'] = upper_edge_radius
args[axis + '0'] = origin[axcoord] + height/2 - upper_edge_radius
tor_upper_max = tor(name='{} max'.format(axis), **args)
yardasol marked this conversation as resolved.
Show resolved Hide resolved

axis_upper_min = plane(axis, 'upper min', height/2 + origin[axcoord] - upper_edge_radius)
cyl_upper_min = cylinder(axis, 'upper min', x1, origin[axcoord1], x2, origin[axcoord2], r - upper_edge_radius)

corners_upper = (+cyl_upper_min & +tor_upper_max & + axis_upper_min)
yardasol marked this conversation as resolved.
Show resolved Hide resolved

if lower_edge_radius > 0.:
args['a'] = r - lower_edge_radius
args['b'] = lower_edge_radius
args['c'] = lower_edge_radius
args[axis + '0'] = origin[axcoord] - height/2 + lower_edge_radius
tor_lower_min = tor(name='{} min'.format(axis), **args)
yardasol marked this conversation as resolved.
Show resolved Hide resolved

axis_lower_max = plane(axis, 'lower max', origin[axcoord] - height/2 + lower_edge_radius)
cyl_lower_min = cylinder(axis, 'lower min', x1, origin[axcoord1], x2, origin[axcoord2], r - lower_edge_radius)

corners_lower = (+cyl_lower_min & +tor_lower_min & - axis_lower_max)
yardasol marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems as though there is an opportunity to reuse at least some of this code in a modular way (e.g. a function or loop) rather than cut/paste for the upper/lower. The benefit of this kind of reuse is for consistency/maintainability and not for performance.

if not(corners_lower is None) and not(corners_upper is None):
yardasol marked this conversation as resolved.
Show resolved Hide resolved
corners = corners_lower | corners_upper
elif corners_lower is None:
corners = corners_upper
else:
corners = corners_lower

prism = prism & ~corners

return prism


yardasol marked this conversation as resolved.
Show resolved Hide resolved

def rectangular_prism(width, height, axis='z', origin=(0., 0.),
boundary_type='transmission', corner_radius=0.):
Expand Down
44 changes: 44 additions & 0 deletions tests/unit_tests/test_geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,50 @@ def test_get_by_name():
assert not univs


def test_cyl_prism():
cyl_prism = openmc.model.cylindrical_prism(2,
5,
origin=(-1,1,3))
# clear checks
assert (-1, 1, 3) in cyl_prism # center
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 it would be beneficial to use the openmc.lib.find_cell method to check that the C++ code is also getting the correct result too since it's what will affect transport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea!

assert (-1,0.5, 1) in cyl_prism # lower
assert (0.5, 1, 5) in cyl_prism # upper
assert (-1, 1, 6) not in cyl_prism
assert (-1, 1, 0) not in cyl_prism
assert (-2, -1, 3) not in cyl_prism
# edge checks
assert (-1, 3.01, 3) not in cyl_prism
assert (-1, 2.99, 3) in cyl_prism
assert (1.01, 1, 3) not in cyl_prism
assert (0.99, 1, 3) in cyl_prism
assert (-2.4, 1, 5.51) not in cyl_prism
assert (-2.4, 1, 5.49) in cyl_prism
assert (-2.4, 1, 0.49) not in cyl_prism
assert (-2.4, 1, 0.51) in cyl_prism

rounded_cyl_prism = openmc.model.cylindrical_prism(2,
5,
origin=(-1,1,3),
upper_edge_radius=1.6,
lower_edge_radius=1.6)
# clear checks
assert (-1, 1, 3) in rounded_cyl_prism # center
assert (-1,0.5, 1) in rounded_cyl_prism # lower
assert (0.5, 1, 5) in rounded_cyl_prism # upper
assert (-1, 1, 6) not in rounded_cyl_prism
assert (-1, 1, 0) not in rounded_cyl_prism
assert (-2, -1, 3) not in rounded_cyl_prism
# edge checks
assert (-1, 3.01, 3) not in rounded_cyl_prism
assert (-1, 2.99, 3) in rounded_cyl_prism
assert (1.01, 1, 3) not in rounded_cyl_prism
assert (0.99, 1, 3) in rounded_cyl_prism
assert (-2.4, 1, 5.51) not in rounded_cyl_prism
assert (-2.4, 1, 5.49) not in rounded_cyl_prism
assert (-2.4, 1, 0.49) not in rounded_cyl_prism
assert (-2.4, 1, 0.51) not in rounded_cyl_prism


def test_hex_prism():
hex_prism = openmc.model.hexagonal_prism(edge_length=5.0,
origin=(0.0, 0.0),
Expand Down