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

Component density differs if you create component through blueprints vs programatically #819

Closed
onufer opened this issue Aug 5, 2022 · 7 comments · Fixed by #841
Closed
Assignees
Labels
bug Something is wrong: Highest Priority

Comments

@onufer
Copy link
Member

onufer commented Aug 5, 2022

Component density differs if you create component through blueprints vs programmatically .

This is due to having to remember to call applyHotHeightDensityReduction() if you do so programmatically. This is not great because if you forget to call it, your component wont be equal to the 3D density. We should strive to have components density always equal to 3D density. We used to do this on component construction, but #657 changed this.

I don't think that it needed to, the math just might be a little different for axial expansion.

#818 writes a unit tests demonstrating this behavior

@onufer
Copy link
Member Author

onufer commented Aug 5, 2022

Thoughts @john-science, @albeanth ?

@onufer onufer mentioned this issue Aug 5, 2022
7 tasks
@john-science john-science changed the title Component density differs if you create component through bluepritns vs programatically Component density differs if you create component through blueprints vs programatically Aug 5, 2022
@john-science john-science added the bug Something is wrong: Highest Priority label Aug 5, 2022
@john-science
Copy link
Member

@albeanth Okay, my read of the situation is this is a bug. Unless you disagree, I am assigning the bug to you, because it has the phrase applyHot in it. Holler at me if I'm wrong.

@albeanth
Copy link
Member

albeanth commented Aug 5, 2022

Looking into it.

@albeanth
Copy link
Member

albeanth commented Aug 5, 2022

Component density differs if you create component through blueprints vs programmatically .

I believe the issue you've identified here boils down to an inconsistency between the default behavior of the Component constructor and case setting, inputHeightsConsideredHot (introduced with #657). Testing on my end shows that if the case settings have inputHeightsConsideredHot: False, then the number densities from a Component built via blueprints and programmatically are the same.

Some more details:

As of now (after #804), the constructor of Component expects that the "height" of the component is cold. So when a Component is built programmatically, the number densities are computed with the adjustment for hot radial dimensions only. However, the default behavior of the blueprints expects the "height" of the Component to be hot (via inputHeightConsideredHot: True) , and therefore the number densities are adjusted for both hot radial and hot axial conditions via a call to applyHotHeightDensityReduction. This inconsistency results in the discrepancy you've identified.

So an option for a fix: Have the default behavior of the Component constructor compute number densities that assume hot radial and hot axial dimensions to match the default behavior of Components built through the blueprints.

Some impacts of this change:

  • If the case setting inputHeightsConsideredHot: False, then we will need to recompute/adjust Component number densities to account for a cold axial height when building from blueprints.
  • If a user wants to build a Component programatically that has a cold "height", they will need to manually call the method to adjust the number densities.
    • note: this is the same scenario you are experiencing now, just reversed.

@john-science
Copy link
Member

I would be curious to know how this problem relates to this docstring:

def density(self, Tk: float = None, Tc: float = None) -> float:
"""
Return density that preserves mass when thermally expanded in 2D.
Warning
-------
This density will not agree with the component density since this method only expands in 2 dimensions.
The component has been manually expanded axially with the manually entered block hot height.

This has been in our codebase for at least 3 years.

@albeanth
Copy link
Member

albeanth commented Aug 5, 2022

I would be curious to know how this problem relates to this docstring:

@keckler has pointed this out as well in #770 and it feels related to #820 .

I'll spend some time on this today.

@onufer
Copy link
Member Author

onufer commented Aug 5, 2022

We should strive to have components density always equal to 3D density.

If we dont than we have to update a whole bunch of doc strings and how the framework wroks.

@albeanth, this is not related to #820 . There are 2 separate issues read through #818 comments.

#820 are very small differences, much less than expansion

@onufer onufer mentioned this issue Aug 16, 2022
7 tasks
@albeanth albeanth linked a pull request Aug 16, 2022 that will close this issue
7 tasks
onufer added a commit that referenced this issue Aug 17, 2022
- fixed an Issue (#819) where components initialized through the constructor would have
  different density than those created through blueprints.
scottyak pushed a commit to scottyak/armi that referenced this issue Oct 27, 2022
- fixed an Issue (terrapower#819) where components initialized through the constructor would have
  different density than those created through blueprints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants