Allow axial expansion related classes to be more extensible
I'm working on some internal axial expansion work and finding myself needing to modify some of the functionality on the base class. In some cases I can call super and be good, but in other places I need a bit more control. I've got some partially developed code that I wanted to present in a ticket before fully submitting the PR. Estimating 10/6 for draft PR.
ExpanionData.computeThermalExpansionFactors
One example is modifications to how ExpansionData computes expansion factors for a given component. The current logic for this method is fully encased in a single method
https://github.com/terrapower/armi/blob/34619dfa7991454d45d5d61e9002bea766a353a3/armi/reactor/converters/axialExpansionChanger/expansionData.py#L174-L190
but there isn't a simple way to drop in more bespoke expansion into this method without rewriting the entire routine. So, I propose cracking this method specifically into one primary method and two helper methods, more or less like
def computeThermalExpansionFactors(self):
for b in self._a:
self._blockThermalExpansionFactors(b) # name TBD
def _blockThermalExpansionFactors(self, b):
for c in getSolidComponents(b):
self._componentThermalExpansionFactors(c)
def _componentThermalExpansionFactors(self, c):
# more or less the internals of the current implementation
Assembly axial link check
The logic for determining if two components are axially linked is vital for determining how the tops and bottoms of blocks move through expansion. This logic is performed in _determinedLinked and isn't really overridable from within
https://github.com/terrapower/armi/blob/34619dfa7991454d45d5d61e9002bea766a353a3/armi/reactor/converters/axialExpansionChanger/assemblyAxialLinkage.py#L186-L200
I propose a staticmethod that points back to that function. Subclasses can use or override this method if they need additional logic for determining component linking.
Small data class for axial links
The data structure for the linking between blocks and between components is dict[T, list[T | None, T | None]] so there's lines like
https://github.com/terrapower/armi/blob/34619dfa7991454d45d5d61e9002bea766a353a3/armi/reactor/converters/axialExpansionChanger/axialExpansionChanger.py#L325
where it may not be immediately clear (outside of some neighboring comments) what position [0] in this returned value represents.
I would like to add a small dataclass like
class Link:
lower: T | None
upper: T | None
(type T standin) that would make that above code snippet (and others) look like
b.p.zbottom = self.linked.linkedBlocks[b].lower.p.ztop
that's more readable in my opinion.
Other considerations
- Provide an iterator form of
getSolidComponentssince we mostly use it for iteration - #1915 - Combine check for zero-valued and negative expansion factors - https://github.com/terrapower/armi/pull/1861#discussion_r1751058614
- https://github.com/terrapower/armi/issues/1453
Whatever you end up implementing here (I did not read this ticket in extreme detail), please make sure you consider this PR: https://github.com/terrapower/armi/pull/1376
That PR was closed because for whatever reason we haven't pulled the trigger on it, and it became stale. But we very much intend to merge it or something similar to it into the framework in the near future.
Maybe file that under your "Other considerations" section above.
That PR was closed because for whatever reason we haven't pulled the trigger on it, and it became stale. But we very much intend to merge it or something similar to it into the framework in the near future.
@keckler don't you worry! It'll come back ~soon~! It's on the (edge of the) radar
Whatever you end up implementing here (I did not read this ticket in extreme detail), please make sure you consider this PR: https://github.com/terrapower/armi/pull/1376
I think this will introduce functionality to help improve / extend the component linking. I'll make a comment on the changes I think will be useful and we can chat about some solutions or workarounds
@drewj-tp what you propose for computing the thermal expansion factors sounds good to me.
for assembly axial link check -- if you are proposing splitting up _getLinkedComponents into smaller methods for extensibility improvements, cool. yeah that would be nice.
for the data class -- sure. Yeah, that would be nice. You are effectively replacing the 0/1 indexing for lower/upper; which is undoubtedly more readable.