Repository: terrapower/armi

Issue:
Some material modifications are allowed to be the wrong length
We currently have a check that tries to ensure that material modifications in the blueprints are of the same length as the number of blocks in that assembly:

https://github.com/terrapower/armi/blob/db8dc8c28fbea8614cac6c2d9b39a8c624838719/armi/reactor/blueprints/assemblyBlueprint.py#L221-L243

There is apparently a corner case that isn't caught. 

If you have a `by block` and `by component` material modification with the same name on a given block, only one of those will be checked. This is because the method above builds a dict of which things to check, and the same dict keys get reused for `by block` and `by component` mods. So the first one gets overwritten by the second.

Discussion:

Patch:
diff --git a/armi/reactor/blueprints/assemblyBlueprint.py b/armi/reactor/blueprints/assemblyBlueprint.py
index d7ff66f4..0eed90f0 100644
--- a/armi/reactor/blueprints/assemblyBlueprint.py
+++ b/armi/reactor/blueprints/assemblyBlueprint.py
@@ -25,10 +25,10 @@ import yamlize
 from armi import getPluginManagerOrFail
 from armi import runLog
 from armi.reactor import assemblies
-from armi.reactor.flags import Flags
+from armi.reactor import grids
 from armi.reactor import parameters
 from armi.reactor.blueprints import blockBlueprint
-from armi.reactor import grids
+from armi.reactor.flags import Flags
 from armi.settings.fwSettings.globalSettings import CONF_INPUT_HEIGHTS_HOT
 
 
@@ -247,8 +247,7 @@ class AssemblyBlueprint(yamlize.Object):
 
         Returns
         -------
-        bool
-            Result of the check
+        bool: Result of the check
         """
         return bool(value != "" and value is not None)
 
@@ -280,29 +279,35 @@ class AssemblyBlueprint(yamlize.Object):
         b.setB10VolParam(cs[CONF_INPUT_HEIGHTS_HOT])
         return b
 
-    def _checkParamConsistency(self):
+    def _checkParamConsistency(self) -> None:
         """Check that the number of block params specified is equal to the number of blocks specified."""
+        # general things to check
         paramsToCheck = {
             "mesh points": self.axialMeshPoints,
             "heights": self.height,
             "xs types": self.xsTypes,
         }
 
-        for mod in [self.materialModifications] + list(
-            self.materialModifications.byComponent.values()
-        ):
-            for modName, modList in mod.items():
-                paramName = "material modifications for {}".format(modName)
+        # check by-block mat mods
+        for modName, modList in self.materialModifications.items():
+            paramName = f"mat mod for {modName}"
+            paramsToCheck[paramName] = modList
+
+        # check by-component mat mods
+        for comp in self.materialModifications.byComponent.values():
+            for modName, modList in comp.items():
+                paramName = f"material modifications for {modName}"
                 paramsToCheck[paramName] = modList
 
+        # perform the check
         for paramName, blockVals in paramsToCheck.items():
             if len(self.blocks) != len(blockVals):
-                raise ValueError(
-                    "Assembly {} had {} blocks, but {} {}. These numbers should be equal. "
-                    "Check input for errors.".format(
-                        self.name, len(self.blocks), len(blockVals), paramName
-                    )
+                msg = (
+                    f"Assembly {self.name} had {len(self.blocks)} block(s), but {len(blockVals)} "
+                    f"'{paramName}'. These numbers should be equal. Check input for errors."
                 )
+                runLog.error(msg)
+                raise ValueError(msg)
 
 
 for paramDef in parameters.forType(assemblies.Assembly).inCategory(
diff --git a/doc/release/0.4.rst b/doc/release/0.4.rst
index 973d91cf..6f9d3500 100644
--- a/doc/release/0.4.rst
+++ b/doc/release/0.4.rst
@@ -43,6 +43,7 @@ Bug Fixes
 #. Update height of fluid components after axial expansion (`PR#1828 <https://github.com/terrapower/armi/pull/1828>`_)
 #. Material theoretical density is serialized to and read from database. (`PR#1852 <https://github.com/terrapower/armi/pull/1852>`_)
 #. Removed broken and unused column in ``summarizeMaterialData``. (`PR#1925 <https://github.com/terrapower/armi/pull/1925>`_)
+#. Fixed edge case in ``assemblyBlueprint._checkParamConsistency()``. (`PR#1928 <https://github.com/terrapower/armi/pull/1928>`_)
 #. TBD
 
 Quality Work


Test patch:
diff --git a/armi/reactor/blueprints/tests/test_materialModifications.py b/armi/reactor/blueprints/tests/test_materialModifications.py
index 00d45d92..a6487f90 100644
--- a/armi/reactor/blueprints/tests/test_materialModifications.py
+++ b/armi/reactor/blueprints/tests/test_materialModifications.py
@@ -180,7 +180,20 @@ assemblies:
         u = fuelComponent.getMass("U")
         assert_allclose(0.50, u235 / u)
 
-    def test_invalid_component_modification(self):
+    def test_materialModificationLength(self):
+        """If the wrong number of material modifications are defined, there is an error."""
+        with self.assertRaises(ValueError):
+            _a = self.loadUZrAssembly(
+                """
+        material modifications:
+            by component:
+                fuel1:
+                    U235_wt_frac: [0.2]
+            U235_wt_frac: [0.11, 0.22, 0.33, 0.44]
+            """
+            )
+
+    def test_invalidComponentModification(self):
         with self.assertRaises(ValueError):
             _a = self.loadUZrAssembly(
                 """
@@ -188,10 +201,10 @@ assemblies:
             by component:
                 invalid component:
                     U235_wt_frac: [0.2]
-        """
+            """
             )
 
-    def test_zr_wt_frac_modification(self):
+    def test_zrWtFracModification(self):
         a = self.loadUZrAssembly(
             """
         material modifications:
@@ -203,7 +216,7 @@ assemblies:
         zr = fuelComponent.getMass("ZR")
         assert_allclose(0.077, zr / totalMass)
 
-    def test_both_u235_zr_wt_frac_modification(self):
+    def test_bothU235ZrWtFracModification(self):
         a = self.loadUZrAssembly(
             """
         material modifications: