-
Notifications
You must be signed in to change notification settings - Fork 91
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
Custom densities #1745
Custom densities #1745
Conversation
mat = materials.resolveMaterialClassByName(self.material)() | ||
if not isinstance(mat, materials.Custom): | ||
densityRatio = ( | ||
blueprint.customIsotopics[self.isotopics].density |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we always have a density? i thought some custom isotopic input options maybe didnt specify density. maybe they all eventually resolve it though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the custom isotopics entry can be specified without a density, which is why line 238 checks for it. But if customIsotopics
exists then it's attributes are initialized to None
At what temperature is the entered density supposed to correspond to? |
This PR should probably update this section of the docs: |
The density is set at component construction so it should be the cold dimension unless the blueprint was specified to be at hot dimensions. |
I suppose I am asking for this to be outlined somewhere, for instance in the blueprints documentation. |
I would love a unit test that actually tests the new feature you are building here. Show me the goal in your unit test, and show me the edge cases. And example edge case might be proving that this PR fixes the bug in #1272. As always, if that's too much of a lift I can help. |
the tutorial input was assigning custom isotopics to a component made of void material. This was being used to check that warnings were being logged that the material properties were ignored. Allowing isotopics to be assigned to Void material is not allowed anymore and an error (not a warning) is thrown. The testing for material properties being added to void material have been implemented elsewhere.
Co-authored-by: John Stilley <1831479+john-science@users.noreply.github.com>
|
||
# Check that the density is set correctly on the custom density block, | ||
# and that it is not the same as the original | ||
self.assertAlmostEqual(19.1, fuel2.density()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsculac I am curious about this particular assert. It seems like it might be based on the wrong premise.
fuel2
is uses the uranium isotopic number densities
, which has a density of 19.1. But in that component Tinput=25
while Thot=600
. So after the assembly is constructed and when we are examining it here, shouldn't the fuel's density be decreased from the value of 19.1 (which corresponds to Tinput
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are asking if the custom density logic should be adjusting to the working temperature, probably. When I wrote this PR, I had misunderstood what Tin in the blueprints was used for, see #1818 (comment) for more details. In short, there is an assumption of hot heights in the blueprints unless the option inputHeightsConsideredHot
is False
, so in the default case of inputHeightsConsideredHot
=True the feature in this PR will assign incorrect densities for components with custom isotopics because this feature assumes the blueprints are for cold dimensions.
What is the change?
The ability to specify custom densities for non-custom materials is added in this PR.
Why is the change being made?
Previously, only materials designated "Custom" in the blueprints file were allowed to have custom densities. This change allows perturbation of the component density as the component is generated. This is specifically done to prevent modification of the base material properties, which could have unintended downstream effects.
This PR attempts to resolve #1272. The use case is for adjustment of library materials to account for changes in density during fabrication (e.g. porosity) without having to use workarounds like smear density adjustments.
Checklist
doc
folder.pyproject.toml
.