terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
212 stars 82 forks source link

Custom densities #1745

Closed bsculac closed 3 days ago

bsculac commented 1 week ago

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 https://github.com/terrapower/armi/issues/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

keckler commented 1 week ago

At what temperature is the entered density supposed to correspond to?

keckler commented 1 week ago

This PR should probably update this section of the docs: https://terrapower.github.io/armi/user/inputs.html#custom-isotopics

bsculac commented 1 week ago

At what temperature is the entered density supposed to correspond to?

The density is set at component construction so it should be the cold dimension unless the blueprint was specified to be at hot dimensions.

keckler commented 1 week ago

At what temperature is the entered density supposed to correspond to?

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.

john-science commented 1 week ago

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.