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

Allow pin pitch in HexBlocks to fall back to pitch defined by a grid #1765

Open
keckler opened this issue Jul 4, 2024 · 4 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@keckler
Copy link
Member

keckler commented Jul 4, 2024

The pin pitch is gotten by HexBlock.getPinPitch() by examining the clad and wire dimensions:

armi/armi/reactor/blocks.py

Lines 2420 to 2464 in 8768dec

def getPinPitch(self, cold=False):
"""
Get the pin pitch in cm.
Assumes that the pin pitch is defined entirely by contacting cladding tubes and wire wraps.
Grid spacers not yet supported.
.. impl:: Pin pitch within block is retrievable.
:id: I_ARMI_BLOCK_DIMS6
:implements: R_ARMI_BLOCK_DIMS
This implementation requires that clad and wire Components are present.
If not, an error is raised. If present, the pin pitch is calculated
as the sum of the outer diameter of the clad and outer diameter of
the wire.
Parameters
----------
cold : boolean
Determines whether the dimensions should be cold or hot
Returns
-------
pinPitch : float
pin pitch in cm
"""
try:
clad = self.getComponent(Flags.CLAD)
wire = self.getComponent(Flags.WIRE)
except ValueError:
raise ValueError(
"Block {} has multiple clad and wire components,"
" so pin pitch is not well-defined.".format(self)
)
if wire and clad:
return clad.getDimension("od", cold=cold) + wire.getDimension(
"od", cold=cold
)
else:
raise ValueError(
"Cannot get pin pitch in {} because it does not have a wire and a clad".format(
self
)
)

This implicitly assumes that every HexGrid has a wire wrap. This is not universally true. It also assumes that there is only one type of clad and one type of wire in the block. This is certainly not universally true.

If we look at the parent class method, Block.getPinPitch() will actually check if the block has a grid defined, and it will use that to get the pin pitch within the block:

armi/armi/reactor/blocks.py

Lines 1270 to 1276 in 8768dec

def getPinPitch(self, cold=False):
"""
Return sub-block pitch in blocks.
This assumes the spatial grid is defined by unit steps
"""
return self.spatialGrid.pitch

This implementation on the parent class allows for the pin pitch to be gathered, independent of the assumptions that I outlined above. It just requires the user to define a grid in the blueprints.

I propose that the HexBlock subclass of getPinPitch() falls back to the parent method implementation if it fails to find the pin pitch using the current method. So basically just adding another try/except clause in there, and then pointing to super().getPinPitch(). This would allow for a lot more flexibility in geometric definition.

@jakehader
Copy link
Member

Maybe related #1402?

@keckler
Copy link
Member Author

keckler commented Jul 8, 2024

Maybe related #1402?

Yeah, you're definitely correct, I forgot about that older ticket 😄

I don't think they are exactly the same issue, but probably resolving one will resolve them both.

@john-science john-science added the enhancement New feature or request label Jul 10, 2024
@john-science
Copy link
Member

Thanks, Chris! I see what you mean about the assumed wire wrap, and I like your solution.

I think it will take more time to write sufficient unit tests to prove the solution works, then to actually implement the solution. But that's fine, it still won't be that onerous.

@john-science john-science added the good first issue Good for newcomers label Jul 10, 2024
@drewj-tp
Copy link

See also #252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants