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

Fixing the affect of custom isotopics on material density #1822

Merged
merged 21 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion armi/reactor/blueprints/blockBlueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from armi.reactor.composites import Composite
from armi.reactor.converters import blockConverters
from armi.reactor.flags import Flags
from armi.settings.fwSettings.globalSettings import CONF_INPUT_HEIGHTS_HOT


def _configureGeomOptions():
Expand Down Expand Up @@ -144,7 +145,11 @@ def construct(
filteredMaterialInput, byComponentMatModKeys = self._filterMaterialInput(
materialInput, componentDesign
)
c = componentDesign.construct(blueprint, filteredMaterialInput)
c = componentDesign.construct(
blueprint,
filteredMaterialInput,
cs[CONF_INPUT_HEIGHTS_HOT],
)
components[c.name] = c

# check that the mat mods for this component are valid options
Expand Down
73 changes: 50 additions & 23 deletions armi/reactor/blueprints/componentBlueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def shape(self, shape):
mergeWith = yamlize.Attribute(type=str, default=None)
area = yamlize.Attribute(type=float, default=None)

def construct(self, blueprint, matMods):
def construct(self, blueprint, matMods, inputHeightsConsideredHot):
Copy link
Member

@john-science john-science Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the variable name here is strange. Because "input heights" makes sense if this Component is part of an Assembly in some way, sure.

But Components are a very general idea of the leaf of the data model. They don't have to be in a pin-type reactor.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe this new parameter should have a default value? What do you think?

Copy link
Member

@keckler keckler Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter and its name are directly copied from the case setting of the same name. I tried to reflect that intention in the docstring, but maybe I could be more clear there.

When Jake put up the draft PR, he was originally passing an entire case settings object down to this level. I did away with that because I find it frustrating when I have to whip up an entire cs just for a single boolean.

So I made the name for this parameter exactly the same as the case setting whose value should be passed down here. I thought that made things simpler, but if you think it'd be more clear either (1) with a different name, or (2) if we passed the entire cs, then I am open to those options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With respect to a default value...

I'm not sure how I feel there.

On the one hand, we could potentially make the default be the same as the default for the inputHeightsConsideredHot case setting (True). On the other hand, I really don't like that because the default on that case setting is the opposite of our typical workflow at this point in time (i.e. having axial expansion turned on).

I think my ideal solution would be if there were a way to not have to pass the value via method calls. But the only way I could conceive to do that is to store a cs on the ComponentBlueprint, and that is something I really don't want to do.

Maybe you see a clean way to get around all this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is definitely better to pass one value instead of the entire settings object. For sure. Nice.

I suppose "input dimensions considered hot" would have been a more accurate name for that setting. But the people who created it were definitely thinking of pin-type reactors.

"""Construct a component or group.

.. impl:: User-defined on material alterations are applied here.
Expand All @@ -205,6 +205,17 @@ def construct(self, blueprint, matMods):
The ``applyInputParams()`` method of that material class is then called,
passing in the associated material modifications data, which the material
class can then use to modify the isotopics as necessary.

Parameters
----------
blueprint : Blueprints
Blueprints object containing various detailed information, such as nuclides to model

matMods : dict
Material modifications to apply to the component.

inputHeightsConsideredHot : bool
See the case setting of the same name.
"""
runLog.debug("Constructing component {}".format(self.name))
kwargs = self._conformKwargs(blueprint, matMods)
Expand All @@ -214,7 +225,9 @@ class can then use to modify the isotopics as necessary.
constructedObject = composites.Composite(self.name)
for groupedComponent in group:
componentDesign = blueprint.componentDesigns[groupedComponent.name]
component = componentDesign.construct(blueprint, matMods=dict())
component = componentDesign.construct(
blueprint, {}, inputHeightsConsideredHot
)
# override free component multiplicity if it's set based on the group definition
component.setDimension("mult", groupedComponent.mult)
_setComponentFlags(component, self.flags, blueprint)
Expand All @@ -229,29 +242,33 @@ class can then use to modify the isotopics as necessary.
constructedObject.material.getTD()
)

# set the custom density for non-custom material components after construction
self.setCustomDensity(constructedObject, blueprint, matMods)
self._setComponentCustomDensity(
constructedObject,
blueprint,
matMods,
inputHeightsConsideredHot,
)

return constructedObject

def setCustomDensity(self, constructedComponent, blueprint, matMods):
def _setComponentCustomDensity(
self, comp, blueprint, matMods, inputHeightsConsideredHot
):
"""Apply a custom density to a material with custom isotopics but not a 'custom material'."""
if self.isotopics is None:
# No custom isotopics specified
return

density = blueprint.customIsotopics[self.isotopics].density
if density is None:
densityFromCustomIsotopic = blueprint.customIsotopics[self.isotopics].density
if densityFromCustomIsotopic is None:
# Nothing to do
return

if density <= 0:
if densityFromCustomIsotopic <= 0:
runLog.error(
"A zero or negative density was specified in a custom isotopics input. "
"This is not permitted, if a 0 density material is needed, use 'Void'. "
"The component is {} and the isotopics entry is {}.".format(
constructedComponent, self.isotopics
)
f"The component is {comp} and the isotopics entry is {self.isotopics}."
)
raise ValueError(
"A zero or negative density was specified in the custom isotopics for a component"
Expand All @@ -261,30 +278,40 @@ def setCustomDensity(self, constructedComponent, blueprint, matMods):
if not isinstance(mat, materials.Custom):
# check for some problem cases
if "TD_frac" in matMods.keys():
runLog.warning(
"Both TD_frac and a custom density (custom isotopics) has been specified for "
"material {}. The custom density will override the density calculated using "
"TD_frac.".format(self.material)
runLog.error(
f"Both TD_frac and a custom isotopic with density {blueprint.customIsotopics[self.isotopics]} "
f"has been specified for material {self.material}. This is an overspecification."
)
if not mat.density(Tc=self.Tinput) > 0:
runLog.error(
"A custom density has been assigned to material '{}', which has no baseline "
f"A custom density has been assigned to material '{self.material}', which has no baseline "
"density. Only materials with a starting density may be assigned a density. "
"This comes up e.g. if isotopics are assigned to 'Void'.".format(
self.material
)
"This comes up e.g. if isotopics are assigned to 'Void'."
)
raise ValueError(
"Cannot apply custom densities to materials without density."
)

densityRatio = density / constructedComponent.density()
constructedComponent.changeNDensByFactor(densityRatio)
# Apply a density scaling to account for the temperature change between Tinput
# Thot. There may be a better place in the initialization to determine
# if the block height will be interpreted as hot dimensions, which would
# allow us to not have to pass the case settings down this far
dLL = comp.material.linearExpansionFactor(
Tc=comp.temperatureInC, T0=comp.inputTemperatureInC
)
if inputHeightsConsideredHot:
f = 1.0 / (1 + dLL) ** 2
else:
f = 1.0 / (1 + dLL) ** 3

scaledDensity = comp.density() / f
densityRatio = densityFromCustomIsotopic / scaledDensity
comp.changeNDensByFactor(densityRatio)

runLog.important(
"A custom material density was specified in the custom isotopics for non-custom "
"material {}. The component density has been altered to "
"{}.".format(mat, constructedComponent.density()),
f"material {mat}. The component density has been altered to "
f"{comp.density()} at temperature {comp.temperatureInC} C",
single=True,
)

Expand Down
8 changes: 4 additions & 4 deletions armi/reactor/blueprints/isotopicOptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,10 @@ def apply(self, material):
if self.density is not None:
if not isinstance(material, materials.Custom):
runLog.important(
"A custom density or number densities has been specified for non-custom "
"material {}. The material object's density will not be updated to prevent unintentional "
"density changes across the model. Only custom materials may have a density "
"specified.".format(material),
"A custom isotopic with associated density has been specified for non-`Custom` "
f"material {material}. The reference density of materials in the materials library "
"will not be changed, but the associated components will use the density "
"implied by the custom isotopics.",
single=True,
)
# specifically, non-Custom materials only use refDensity and dLL, mat.customDensity has no effect
Expand Down
3 changes: 2 additions & 1 deletion armi/reactor/blueprints/tests/test_blueprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from armi.utils import directoryChangers
from armi.utils import textProcessors
from armi.reactor.blueprints.gridBlueprint import saveToStream
from armi.settings.fwSettings.globalSettings import CONF_INPUT_HEIGHTS_HOT


class TestBlueprints(unittest.TestCase):
Expand Down Expand Up @@ -612,7 +613,7 @@ def test_topLevelComponentInput(self):
# which is required during construction of a component
design._resolveNuclides(cs)
componentDesign = design.componentDesigns["freefuel"]
topComponent = componentDesign.construct(design, matMods=dict())
topComponent = componentDesign.construct(design, {}, cs[CONF_INPUT_HEIGHTS_HOT])
self.assertEqual(topComponent.getDimension("od", cold=True), 4.0)
self.assertGreater(topComponent.getVolume(), 0.0)
self.assertGreater(topComponent.getMass("U235"), 0.0)
Expand Down
24 changes: 19 additions & 5 deletions armi/reactor/blueprints/tests/test_customIsotopics.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,12 @@ class TestCustomIsotopics(unittest.TestCase):
@classmethod
def setUpClass(cls):
cs = settings.Settings()
cs = cs.modified(newSettings={CONF_XS_KERNEL: "MC2v2"})
cs = cs.modified(
newSettings={
CONF_XS_KERNEL: "MC2v2",
"inputHeightsConsideredHot": False,
}
)

cls.bp = blueprints.Blueprints.load(cls.yamlString)
cls.a = cls.bp.constructAssem(cs, name="fuel a")
Expand Down Expand Up @@ -356,14 +361,19 @@ def test_densitiesAppliedToNonCustomMaterials(self):
# A block with custom density set via number density
fuel8 = self.a[8].getComponent(Flags.FUEL)

dLL = fuel2.material.linearExpansionFactor(Tc=600, T0=25)
# the exponent here is 3 because inputHeightsConsideredHot = False.
# if inputHeightsConsideredHot were True, then we would use a factor of 2 instead
f = 1 / ((1 + dLL) ** 3)

# 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())
self.assertAlmostEqual(19.1 * f, fuel2.density())
john-science marked this conversation as resolved.
Show resolved Hide resolved
self.assertNotAlmostEqual(fuel0.density(), fuel2.density(), places=2)
# Check that the custom density block has the correct material
self.assertEqual("UZr", fuel2.material.name)
# Check that the block with only number densities set has a new density
self.assertAlmostEqual(19.1, fuel8.density())
self.assertAlmostEqual(19.1 * f, fuel8.density())
john-science marked this conversation as resolved.
Show resolved Hide resolved
# original material density should not be changed after setting a custom density component,
# so a new block without custom isotopics and density should have the same density as the original
self.assertAlmostEqual(fuel6.density(), fuel0.density())
Expand All @@ -387,12 +397,16 @@ def test_customDensityLogsAndErrors(self):

# Check for log messages
streamVal = mockLog.getStdout()
self.assertIn("Both TD_frac and a custom density", streamVal, msg=streamVal)
self.assertIn(
"Both TD_frac and a custom isotopic with density",
streamVal,
msg=streamVal,
)
self.assertIn(
"A custom material density was specified", streamVal, msg=streamVal
)
self.assertIn(
"A custom density or number densities has been specified",
"A custom isotopic with associated density has been specified for non-`Custom`",
streamVal,
msg=streamVal,
)
Expand Down