-
Notifications
You must be signed in to change notification settings - Fork 82
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
add component density Tests #818
Conversation
# This should be equal, but block construction calls applyHotHeightDensityReduction | ||
# while programmatic construction allows components to exist in a state where | ||
# their density is not consistent with material density. | ||
self.assertNotAlmostEqual( | ||
clad.getMassDensity(), | ||
programaticClad.getMassDensity(), | ||
delta=biggerDelta, | ||
) |
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.
We probably want to fix this. it is documented in #819
|
||
# when comparing to 3D density, the comparison is not quite correct. | ||
# We need a bigger delta, this will be investigated/fixed in another PR | ||
biggerDelta = 0.001 # g/cc |
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.
The fact that we need this is indicative of a bug. Thankfully the differences are very minor.
#820
I love it when people add tests! Reviewing now. |
I would be curious to know how this problem relates to this docstring: armi/armi/materials/material.py Lines 341 to 348 in bc556fc
This has been in our codebase for at least 3 years. |
@john-science, that doc string indicates that until you fix it it will be off by axial expansion factor. but its not exactly off axial expansion factor, and that is the problem.. It should agree exactly with this one, but it doesn't armi/armi/materials/material.py Lines 357 to 358 in bc556fc
|
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.
I love it when people add more tests.
These two kind of "prove a negative", which is unusual. But they are still really helpful to have around.
- revert renames of applyHotHeightDensityReduction, and applyMaterialMassFracsToNumberDensities - add getMassDensity() function to components
Description
applyHotHeightDensityReduction
, andapplyMaterialMassFracsToNumberDensities
from recent PRgetMassDensity()
function to componentsChecklist
doc
folder.setup.py
.