armi
armi copied to clipboard
Docstring for `Material.density()` is inconsistent with new axial thermal expansion functionality
The docstring for the density() method on the base class speaks completely unawares of the ability for thermal axial expansion:
https://github.com/terrapower/armi/blob/0a9b8570b18cbfe477cc4dda113ca97d0e9e8c8a/armi/materials/material.py#L337-L357
We probably need to add some nuance to it so that is still makes sense going forward as the thermal axial expansion becomes more commonly used.
@albeanth @john-science
@albeanth Thoughts?
We believe this issue was resolved with the density/pseudodensity PR:
https://github.com/terrapower/armi/pull/1163
I actually disagree. The docstring on density() is now fine, but the issue was effectively shifted to the docstring for pseudoDensity():
https://github.com/terrapower/armi/blob/5912372a9c037f8b6caf52749ebe6dc28c06b873/armi/materials/material.py#L420-L440
In particular, "The component has been manually expanded axially with the manually entered block hot height."
This sentence is only sometimes true, depending on a user's modeling choice and whether or not they use inputHeightsConsideredHot.
Okay, @keckler , what do you want the sentence to read? Mind you, not everyone using ARMI will be using the new Axial Thermal Expansion feature, and we want to keep the NOTE on this to be ~one sentence.
My proposal would be:
Warning
-------
The pseudoDensity will not typically agree with the component density since this method only expands in 2 dimensions.
Depending on your use of `inputHeightsConsideredHot` and the current component's temperature,
the pseudoDensity may be a factor of (1 + dLL) different than the density on the component.
In the case of Fluids, density and pseudoDensity are the same as density is not driven by linear expansion, but
rather an explicit density function dependent on Temperature. linearExpansionPercent is zero for a fluid.
I'd want to make sure that @albeanth is good with this modification though.
I'd want to make sure that @albeanth is good with this modification though.
I like it. A minor tweak to consider
Warning
---------
Material.pseudoDensity will not typically agree with Material.density or Component.density
since this methods only expands in 2 dimensions. Depending on your use of
`inputHeightsConsideredHot` and Component.temperatureInC, Material.psuedoDensity may be
a factor of (1+dLL) different than Material.density or Component.density.
In the case of Fluids, Material.density and Material.pseudoDensity are the same ....
Material.linearExpansionPercent is zero ....
I think the See Also could be rewritten as the first comment is innacurate and the second no longer exists as written. An idea:
See Also
---------
:py:meth:`armi.materials.material.Material.density`
:py:meth:`armi.reactor.components.component.Component.density`
Good additions!