Improve Flexibility Of Component Temp Mapping + Bug Fix With Radial NDens Update

[albeanth]

Description

This PR has two features (I know it bends the rules a little bit):

1. Allows users to directly specify a component temperature without having to map a 1D temperature distribution. This is done with a new public method updateComponentTemp.
2. Introduces a new optional parameter that updates the number density of a component when the component temperature is updated.
   • this is more in line with standard ARMI practices of updating radial number densities when setting components to new temperatures via c.setTemperature. The new optional parameter is updateNDensForRadialExp.
   • It was made True by default to provide consistency with other ARMI functionality that updates component temperatures.
   • This also helped identify an existing bug. When updating component volume, c.getArea(cold) expects cold to be a boolean, not a value. The bug never reared its head as a float was passed into getArea (floats evaluate as True). However it is corrected here with the introduction of allowing number densities to be updated for radial expansion/contraction.

---

Checklist

• This PR has only one purpose or idea.
  - There are technically 2 but they go hand in hand.
• Tests have been added/updated to verify that the new/changed code works.

• The code style follows good practices.
• The commit message(s) follow good practices.

• The release notes are up-to-date with any bug fixes or new features.
• The documentation is still up-to-date in the doc folder.
• The dependencies are still up-to-date in setup.py.

[albeanth]

@mgjarrett I would appreciate a review from you as well!

[opotowsky]

Looks great. Code coverage is a go. I think I understand all the changes, too, which means I might be getting somewhere above 0% understanding the axial expansion features.

I also think the PR is still single-purpose. I mean, you need the new parameter for the new method. Two features, single purpose, and @john-science won't inhale any seawater over this.

[mgjarrett]

@albeanth this PR looks great to me. I'm going to merge it with #846 locally and run tests to make sure the two play nicely together.

[mgjarrett]

#846 rebases on #845 with no conflicts, and the tests pass for me locally!

[jakehader]

I still thing that we need a test that demonstrates that densities/number of atoms/masses are being preserved under the conditions when updateNDensForRadialExp is set to True. Since the default has been changed to True for considering radial thermal expansion, I am not sure how the existing tests fit into this. My understanding from @albeanth is that the tests were only written w.r.t. demonstrating the axial thermal expansion in isolation (i.e., updateNDensForRadialExp is False), so where in the tests are we maintaining these checks as wells as introducing new tests for updateNDensForRadialExp set to True?

I'd like to see a single assembly test under both of these conditions as well as some checks on block-wise masses and even volume fractions. I'd also like to see a test that demonstrates this in conjunction with the use of b.setPitch. It seems that if we update the temperature of the block's _pitchDefiningComponent then there needs to be a process where the pitch is either updated automatically, or it should be very clear if the user needs to do this.

[mgjarrett]

@jakehader

I still thing that we need a test that demonstrates that densities/number of atoms/masses are being preserved under the conditions when updateNDensForRadialExp is set to True. Since the default has been changed to True for considering radial thermal expansion, I am not sure how the existing tests fit into this. My understanding from @albeanth is that the tests were only written w.r.t. demonstrating the axial thermal expansion in isolation (i.e., updateNDensForRadialExp is False), so where in the tests are we maintaining these checks as wells as introducing new tests for updateNDensForRadialExp set to True?

@onufer suggested a very similar unit test last week, which was the impetus for #846. test_coldAssemblyExpansion runs thermal expansion with updateNDensForRadialExp: true and verifies density by comparing component.getMassDensity() with component.material.density3(component.temperatureInC): https://github.com/mgjarrett/armi/blob/ed620dfdb9757fe4182c6f51a4cfa3eecd928c2b/armi/reactor/converters/tests/test_axialExpansionChanger.py#L827-L839

I'd like to see a single assembly test under both of these conditions as well as some checks on block-wise masses and even volume fractions. I'd also like to see a test that demonstrates this in conjunction with the use of b.setPitch. It seems that if we update the temperature of the block's _pitchDefiningComponent then there needs to be a process where the pitch is either updated automatically, or it should be very clear if the user needs to do this.

Block-wise masses are only conserved for target components; this is tested in test_TargetComponentMassConservation here: https://github.com/albeanth/armi/blob/7cc7956aef091ac51ac10984d9756dd0988842da/armi/reactor/converters/tests/test_axialExpansionChanger.py#L413-L431

Other components do not have mass conserved. Either mass or density can be conserved for non-target components, but not both. It was initially implemented as mass, but #846 is changing it to density, which was the original plan. Mass is still conserved over the full assembly, which is tested here: test_PrescribedExpansionContractionConservation: https://github.com/albeanth/armi/blob/7cc7956aef091ac51ac10984d9756dd0988842da/armi/reactor/converters/tests/test_axialExpansionChanger.py#L380-L411

I don't know of a test that verifies volume fractions are conserved. This would probably be a good add.

We also don't have any test to verify number densities after using b.setPitch that I am aware of.

[albeanth]

@mgjarrett thanks for taking on the response to @jakehader !

I don't know of a test that verifies volume fractions are conserved. This would probably be a good add.

This is true. We do not have any tests to verify volume fractions. In the interest of atomic PRs, this feels worthy of a separate issue + PR. I can take this on.

We also don't have any test to verify number densities after using b.setPitch that I am aware of.

This is also true. This feels like it should be in blocks.py or elsewhere outside of the axial expansion changer and this PR. A good idea nonetheless.

[opotowsky]

I vote enthusiastically for the multiple-PR approach, since they will be easier to review and this PR is needed with some urgency for a plugin

[jakehader]

This looks good! As for testing the pitch or tying pitch changes to a specific component this is outside of the scope of this change. I grepped around the code and don't see any tests for core::setPitchUniform or block.setPitch that tests the conservation of solid component mass. I would like to see a separate PR that adds testing to these two methods (on the reactor core is fine) with and without the axial expansion changer.

armi/armi/reactor/reactors.py

Line 2143 in 86dc4d8

def setPitchUniform(self, pitchInCm):

For example:

• Initialize reactor core, shrink pitch, check that solid masses are conserved and liquid masses (coolant) reduce. Grow pitch back to original, check that solid masses and liquid masses return to the normal state.
• Initialize reactor core model, use axial expansion changer, repeat above.

I want to just check that (1) the pitch change is being propagated correctly at that there are tests for this and (2) that the pitch change with thermal expansion doesn't run into issues if it is called after the axial expansion changer and that the results are correct in terms of changes in masses and volume fractions of each component (see: b.getVolumeFractions())

I will make a new issue to work on based on this comment and then land this!