Allow pin pitch in `HexBlock`s to fall back to pitch defined by a grid

[keckler]

The pin pitch is gotten by HexBlock.getPinPitch() by examining the clad and wire dimensions: https://github.com/terrapower/armi/blob/8768dec747f288a2abd537c209c08e6c0055d367/armi/reactor/blocks.py#L2420-L2464

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: https://github.com/terrapower/armi/blob/8768dec747f288a2abd537c209c08e6c0055d367/armi/reactor/blocks.py#L1270-L1276

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]

Maybe related #1402?

[keckler]

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]

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.

[drewj-tp]

See also #252

[john-science]

Juuuust waiting for someone to take the ticket. :)

[drewj-tp]

Plan is going to be

1. Block.getPinPitch remains the same: pull self.spatialGrid.pitch
2. HexBlock.getPinPitch will not exist, instead falling back to Block.getPinPitch
3. But, we need a way for HexBlock.autoCreateSpatialGrids to know the pitch. In that case, I will move the current HexBlock.getPinPitch logic into a separate method, maybe HexBlock.inferPinPitch (?)

[john-science]

@drewj-tp I am reviewing my release plan. Do you think you will be able to do this ticket in the next, say, two months?

[drewj-tp]

Not likely but I can be available in a pinch and/or to review

[keckler]

FYI I am working on this as part of a different ticket.

[keckler]

Plan is going to be

1. Block.getPinPitch remains the same: pull self.spatialGrid.pitch
2. HexBlock.getPinPitch will not exist, instead falling back to Block.getPinPitch
3. But, we need a way for HexBlock.autoCreateSpatialGrids to know the pitch. In that case, I will move the current HexBlock.getPinPitch logic into a separate method, maybe HexBlock.inferPinPitch (?)

The problem with this plan is a little bit subtle. Right now, HexBlock.getPinPitch() defaults to using hot dimensions to calculate the pitch. This means that all over the place, people are getting the pin pitch at the hot temperature. On the other hand, reaching into the spatial grid to get the pitch parameter will only ever give you the cold pitch, because right now the spatial grid is not updated as temperatures change.

So we are in a bit of a pickle here...

On the one hand, there currently exists a problem in that HexBlock.getPinPitch() will attempt to use hot dimensions to calculate the pitch. However HexBlock.getPinPitch() is not extensible to mixed assemblies if it relies on checking the dimensions of specific components. This method also uses a close-packed assumption, which, even at cold temperatures, may be different than what is defined in the block's grid (see #1402 ).

On the other hand, Block.getPinPitch() currently relies on the block's spatial grid, and it cannot get a pin pitch that is updated to the current temperature. This is problematic because this is something that people apparently want (or at least that's what they're getting right now).

The final problem is that the behavior is inconsistent between different types of blocks. For HexBlocks, calling getPinPitch() would give you the expanded pin pitch, but for any other type of block (e.g. cartesian), you'd get the cold pin pitch.

So basically, this is a very big mess right now. I think the only way to resolve this would be to split the capability into two methods, like getClosePackedPinPitch(cold=False), which checks the component's dimensions, and getBlueprintsPinPitch(), which just looks at the blueprints from the grid.

[drewj-tp]

🥒 Hmm.

For a "cold" pitch, the grids will work. So HexBlock.getPinPitch(cold=True) could look up the spatial grid pitch.

As pins thermally expand, potentially non-uniformly, does that break the concept of a pitch? Like we don't have an even spacing between all pins because some are expanding differently due to different local composition, materials, temperatures. So at best we're looking for an average pitch at hot temperatures.

Could we apply some averaged thermal expansion coefficient to the grid pitch?

[john-science]

I think the only way to resolve this would be to split the capability into two methods, like getClosePackedPinPitch(cold=False), which checks the component's dimensions, and getBlueprintsPinPitch(), which just looks at the blueprints from the grid.

I think something like this is the correct solution.

getPinPitch() could certainly take a cold=True or cold=False . That would give users flexibility they don't currently have.

Multiple types of assemblies is certainly a new complexity. So we have to support that. However, we are adding a new feature to our reactor and our code base so your statement "this is a very big mess right now." I think is hyperbolic. We add a new feature, and there is going to be work to support it. That's to be expected.