Repository: terrapower/armi
Pull request #1392 (2023-08-17 01:35:50)

Fixing some miscellaneous TODOs
## What is the change?

There are several `TODO`s in the ARMI codebase. I found some that were low-hanging fruit to fix or test/delete if they were defunct.

## Why is the change being made?

Our docs say we shouldn't keep `TODO`s in the codebase. Also, some of these were questions that needed testing, so I just did the testing.

There are more left, but perfect is the enemy of better.

---

## Checklist

<!--
    You (the pull requester) should put an `x` in the boxes below you have completed.
    If you're unsure about any of them, don't hesitate to ask. We're here to help!
    Learn what a "good PR" looks like here:
    https://terrapower.github.io/armi/developer/tooling.html#good-pull-requests
-->

- [X] This PR has only one purpose or idea.
- [X] Tests have been added/updated to verify that the new/changed code works.

<!-- Check the code quality -->

- [X] The code style follows [good practices](https://terrapower.github.io/armi/developer/standards_and_practices.html).
- [X] The commit message(s) follow [good practices](https://terrapower.github.io/armi/developer/tooling.html).

<!-- Check the project-level cruft -->

- [X] The [release notes](https://terrapower.github.io/armi/release/index.html) (location `doc/release/0.X.rst`) are up-to-date with any important changes.
- [X] The documentation is still up-to-date in the `doc` folder.
- [X] The dependencies are still up-to-date in `setup.py`.



Patch:
diff --git a/armi/__init__.py b/armi/__init__.py
index a3471516..dff87ddd 100644
--- a/armi/__init__.py
+++ b/armi/__init__.py
@@ -307,7 +307,7 @@ def configure(app: Optional[apps.App] = None, permissive=False):
     pm = app.pluginManager
     parameters.collectPluginParameters(pm)
     parameters.applyAllParameters()
-    flags.registerPluginFlags(pm)
+    _app.registerPluginFlags()
 
 
 def applyAsyncioWindowsWorkaround() -> None:
diff --git a/armi/apps.py b/armi/apps.py
index 1e748312..bce6cd63 100644
--- a/armi/apps.py
+++ b/armi/apps.py
@@ -75,6 +75,7 @@ class App:
         For a description of the things that an ARMI plugin can do, see the
         :py:mod:`armi.plugins` module.
         """
+        self._pluginFlagsRegistered: bool = False
         self._pm: Optional[pluginManager.ArmiPluginManager] = None
         self._paramRenames: Optional[Tuple[Dict[str, str], int]] = None
         self.__initNewPlugins()
@@ -228,6 +229,24 @@ class App:
             self._paramRenames = renames, self._pm.counter
         return renames
 
+    def registerPluginFlags(self):
+        """
+        Apply flags specified in the passed ``PluginManager`` to the ``Flags`` class.
+
+        See Also
+        --------
+        armi.plugins.ArmiPlugin.defineFlags
+        """
+        if self._pluginFlagsRegistered:
+            raise RuntimeError(
+                "Plugin flags have already been registered. Cannot do it twice!"
+            )
+
+        for pluginFlags in self._pm.hook.defineFlags():
+            Flags.extend(pluginFlags)
+
+        self._pluginFlagsRegistered = True
+
     def registerUserPlugins(self, pluginPaths):
         r"""
         Register additional plugins passed in by importable paths.
diff --git a/armi/bookkeeping/db/databaseInterface.py b/armi/bookkeeping/db/databaseInterface.py
index 8fb4e288..d49f04ae 100644
--- a/armi/bookkeeping/db/databaseInterface.py
+++ b/armi/bookkeeping/db/databaseInterface.py
@@ -281,7 +281,6 @@ class DatabaseInterface(interfaces.Interface):
                 "The cycle history up to the restart cycle/node must be equivalent."
             )
 
-    # TODO: The use of "yield" here is suspect.
     def _getLoadDB(self, fileName):
         """
         Return the database to load from in order of preference.
diff --git a/armi/bookkeeping/db/layout.py b/armi/bookkeeping/db/layout.py
index 199c4725..c5bb9249 100644
--- a/armi/bookkeeping/db/layout.py
+++ b/armi/bookkeeping/db/layout.py
@@ -356,7 +356,7 @@ class Layout:
             elif issubclass(Klass, Core):
                 comp = Klass(name)
             elif issubclass(Klass, Component):
-                # TODO: init all dimensions to 0, they will be loaded and assigned after load
+                # init all dimensions to 0, they will be loaded and assigned after load
                 kwargs = dict.fromkeys(Klass.DIMENSION_NAMES, 0)
                 kwargs["material"] = material
                 kwargs["name"] = name
@@ -851,18 +851,11 @@ def replaceNonsenseWithNones(data: numpy.ndarray, paramName: str) -> numpy.ndarr
             if isNone[i].all():
                 result[i] = None
             elif isNone[i].any():
-                # TODO: This is not symmetric with the replaceNonesWithNonsense impl.
-                # That one assumes that Nones apply only at the highest dimension, and
-                # that the lower dimensions will be filled with the magic None value.
-                # Non-none entries below the top level fail to coerce to a serializable
-                # numpy array and would raise an exception when trying to write. TL;DR:
-                # this is a dead branch until the replaceNonesWithNonsense impl is more
-                # sophisticated.
+                # This is the meat of the logic to replace "nonsense" with None.
                 result[i] = numpy.array(data[i], dtype=numpy.dtype("O"))
                 result[i][isNone[i]] = None
             else:
                 result[i] = data[i]
-
     else:
         result = numpy.ndarray(data.shape, dtype=numpy.dtype("O"))
         result[:] = data
diff --git a/armi/bookkeeping/memoryProfiler.py b/armi/bookkeeping/memoryProfiler.py
index 5997249f..6a1053fe 100644
--- a/armi/bookkeeping/memoryProfiler.py
+++ b/armi/bookkeeping/memoryProfiler.py
@@ -307,11 +307,7 @@ class InstanceCounter:
 
         self.ids.add(itemId)
         if self.reportSize:
-            try:
-                self.memSize += sys.getsizeof(item)
-            except:  # noqa: bare-except
-                # TODO: Does this happen?
-                self.memSize = float("nan")
+            self.memSize += sys.getsizeof(item)
         self.count += 1
         return True
 
diff --git a/armi/physics/thermalHydraulics/parameters.py b/armi/physics/thermalHydraulics/parameters.py
index 16fdc272..88e6384e 100644
--- a/armi/physics/thermalHydraulics/parameters.py
+++ b/armi/physics/thermalHydraulics/parameters.py
@@ -268,7 +268,7 @@ def _getBlockParams():
         )
 
     with pDefs.createBuilder(
-        default=0.0, categories=["thermal hydraulics", "mongoose"], saveToDB=True
+        default=None, categories=["thermal hydraulics", "mongoose"], saveToDB=True
     ) as pb:
 
         pb.defParam(
@@ -285,6 +285,9 @@ def _getBlockParams():
             location=ParamLocation.TOP | ParamLocation.EDGES,
         )
 
+    with pDefs.createBuilder(
+        default=0, categories=["thermal hydraulics", "mongoose"], saveToDB=True
+    ) as pb:
         pb.defParam(
             "THhotChannel",
             units=units.UNITLESS,
diff --git a/armi/reactor/blockParameters.py b/armi/reactor/blockParameters.py
index 06ac8d02..478c2cda 100644
--- a/armi/reactor/blockParameters.py
+++ b/armi/reactor/blockParameters.py
@@ -704,7 +704,7 @@ def getBlockParameterDefinitions():
 
     with pDefs.createBuilder(
         default=0.0,
-        location=ParamLocation.AVERAGE,
+        location=ParamLocation.VOLUME_INTEGRATED,
         categories=[
             "reactivity coefficients",
             "spatially dependent",
diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py
index f068cc00..1b232276 100644
--- a/armi/reactor/blocks.py
+++ b/armi/reactor/blocks.py
@@ -562,6 +562,8 @@ class Block(composites.Composite):
         newEnrich : float
             New U-235 enrichment in mass fraction
 
+        Notes
+        -----
         completeInitialLoading must be run because adjusting the enrichment actually
         changes the mass slightly and you can get negative burnups, which you do not want.
         """
@@ -800,7 +802,6 @@ class Block(composites.Composite):
 
         hmDens = bolBlock.getHMDens()  # total homogenized heavy metal number density
         self.p.nHMAtBOL = hmDens
-
         self.p.molesHmBOL = self.getHMMoles()
         self.p.puFrac = (
             self.getPuMoles() / self.p.molesHmBOL if self.p.molesHmBOL > 0.0 else 0.0
@@ -811,6 +812,7 @@ class Block(composites.Composite):
             self.p.smearDensity = self.getSmearDensity()
         except ValueError:
             pass
+
         self.p.enrichmentBOL = self.getFissileMassEnrich()
         massHmBOL = 0.0
         sf = self.getSymmetryFactor()
@@ -820,7 +822,9 @@ class Block(composites.Composite):
             # Components have a massHmBOL parameter but not every composite will
             if isinstance(child, components.Component):
                 child.p.massHmBOL = hmMass
+
         self.p.massHmBOL = massHmBOL
+
         return hmDens
 
     def setB10VolParam(self, heightHot):
@@ -1965,13 +1969,6 @@ class HexBlock(Block):
                     pinToDuctGap, self
                 )
             )
-            wire = self.getComponent(Flags.WIRE)
-            wireThicknesses = wire.getDimension("od", cold=False)
-            if pinToDuctGap < wireThicknesses:
-                raise ValueError(
-                    "Gap between pins and duct is {0:.4f} cm in {1} which does not allow room for the wire "
-                    "with diameter {2}".format(pinToDuctGap, self, wireThicknesses)
-                )
         elif pinToDuctGap is None:
             # only produce a warning if pin or clad are found, but not all of pin, clad and duct. We
             # may need to tune this logic a bit
diff --git a/armi/reactor/blueprints/assemblyBlueprint.py b/armi/reactor/blueprints/assemblyBlueprint.py
index b0d80b93..727437ce 100644
--- a/armi/reactor/blueprints/assemblyBlueprint.py
+++ b/armi/reactor/blueprints/assemblyBlueprint.py
@@ -214,7 +214,6 @@ class AssemblyBlueprint(yamlize.Object):
             cs, blueprint, axialIndex, meshPoints, height, xsType, materialInput
         )
 
-        # TODO: remove when the plugin system is fully set up?
         b.completeInitialLoading()
 
         # set b10 volume cc since its a cold dim param
diff --git a/armi/reactor/composites.py b/armi/reactor/composites.py
index 1c3c1abd..92b5ad29 100644
--- a/armi/reactor/composites.py
+++ b/armi/reactor/composites.py
@@ -392,8 +392,7 @@ class ArmiObject(metaclass=CompositeModelType):
         state["parent"] = None
 
         if "r" in state:
-            # TODO: This should never happen, it might make sense to raise an exception.
-            del state["r"]
+            raise RuntimeError("An ArmiObject should never contain the entire Reactor.")
 
         return state
 
diff --git a/armi/reactor/converters/uniformMesh.py b/armi/reactor/converters/uniformMesh.py
index 64569046..a89349ac 100644
--- a/armi/reactor/converters/uniformMesh.py
+++ b/armi/reactor/converters/uniformMesh.py
@@ -1317,7 +1317,7 @@ class ParamMapper:
             if val is None:
                 continue
 
-            if isinstance(val, (list, numpy.ndarray)):
+            if isinstance(val, (tuple, list, numpy.ndarray)):
                 ParamMapper._arrayParamSetter(block, [val], [paramName])
             else:
                 ParamMapper._scalarParamSetter(block, [val], [paramName])
@@ -1327,22 +1327,11 @@ class ParamMapper:
         paramVals = []
         for paramName in paramNames:
             val = block.p[paramName]
-            defaultValue = self.paramDefaults[paramName]
-            valType = type(defaultValue)
-            # Array / list parameters can be have values that are `None`, lists, or numpy arrays. This first
-            # checks if the value type is any of these and if so, the block-level parameter is treated as an
-            # array.
-            if isinstance(valType, (list, numpy.ndarray)) or isinstance(None, valType):
-                if val is None or len(val) == 0:
-                    paramVals.append(None)
-                else:
-                    paramVals.append(numpy.array(val))
-            # Otherwise, the parameter is treated as a scalar, like a float/string/integer.
+            # list-like should be treated as a numpy array
+            if isinstance(val, (tuple, list, numpy.ndarray)):
+                paramVals.append(numpy.array(val) if len(val) > 0 else None)
             else:
-                if val == defaultValue:
-                    paramVals.append(defaultValue)
-                else:
-                    paramVals.append(val)
+                paramVals.append(val)
 
         return numpy.array(paramVals, dtype=object)
 
diff --git a/armi/reactor/flags.py b/armi/reactor/flags.py
index d1e2c8e3..c59775a8 100644
--- a/armi/reactor/flags.py
+++ b/armi/reactor/flags.py
@@ -151,7 +151,6 @@ def _fromStringIgnoreErrors(cls, typeSpec):
     a. multiple-word flags are used such as *grid plate* or
        *inlet nozzle* so we use lookups.
     b. Some flags have digits in them. We just strip those off.
-
     """
 
     def updateMethodIgnoreErrors(typeSpec):
@@ -298,28 +297,6 @@ class InvalidFlagsError(KeyError):
     pass
 
 
-_PLUGIN_FLAGS_REGISTERED = False
-
-
-def registerPluginFlags(pm):
-    """
-    Apply flags specified in the passed ``PluginManager`` to the ``Flags`` class.
-
-    See Also
-    --------
-    armi.plugins.ArmiPlugin.defineFlags
-    """
-    global _PLUGIN_FLAGS_REGISTERED
-    if _PLUGIN_FLAGS_REGISTERED:
-        raise RuntimeError(
-            "Plugin flags have already been registered. Cannot do it twice!"
-        )
-
-    for pluginFlags in pm.hook.defineFlags():
-        Flags.extend(pluginFlags)
-    _PLUGIN_FLAGS_REGISTERED = True
-
-
 # string conversions for multiple-word flags
 # Beware of how these may interact with the standard flag names! E.g., make sure NOZZLE
 # doesn't eat the NOZZLE in INLET_NOZZLE. Make sure that words that would otherwise be a
diff --git a/armi/reactor/grids.py b/armi/reactor/grids.py
index 9ce75545..6c9d7c1c 100644
--- a/armi/reactor/grids.py
+++ b/armi/reactor/grids.py
@@ -1054,8 +1054,7 @@ class Grid:
 
         A tuple is returned so that it is easy to compare pairs of indices.
         """
-        # Regular grids dont really know about ring and position. We can try to see if
-        # their parent does!
+        # Regular grids don't know about ring and position. Check the parent.
         if (
             self.armiObject is not None
             and self.armiObject.parent is not None
@@ -1063,9 +1062,7 @@ class Grid:
         ):
             return self.armiObject.parent.spatialGrid.getRingPos(indices)
 
-        # For compatibility's sake, return __something__. TODO: We may want to just
-        # throw here, to be honest.
-        return tuple(i + 1 for i in indices[:2])
+        raise ValueError("No ring position found, because no spatial grid was found.")
 
     def getAllIndices(self):
         """Get all possible indices in this grid."""
diff --git a/armi/reactor/parameters/parameterDefinitions.py b/armi/reactor/parameters/parameterDefinitions.py
index e1163ebd..458a849f 100644
--- a/armi/reactor/parameters/parameterDefinitions.py
+++ b/armi/reactor/parameters/parameterDefinitions.py
@@ -38,7 +38,7 @@ from armi.reactor.flags import Flags
 from armi.reactor.parameters.exceptions import ParameterError, ParameterDefinitionError
 
 # bitwise masks for high-speed operations on the `assigned` attribute
-# (see http://www.vipan.com/htdocs/bitwisehelp.html)
+# see: https://web.archive.org/web/20120225043338/http://www.vipan.com/htdocs/bitwisehelp.html
 # Note that the various operations are responsible for clearing the flags on the events.
 # These should be interpreted as:
 #   The Parameter or ParameterCollection has been modified SINCE_<time-description>
@@ -70,6 +70,8 @@ class Category:
     * `neutronics` parameters are calculated in a neutronics global flux solve
     * `gamma` parameters are calculated in a fixed-source gamma solve
     * `detailedAxialExpansion` parameters are marked as such so that they are mapped from the uniform mesh back to the non-uniform mesh
+    * `reactivity coefficients` parameters are related to reactivity coefficient or kinetics parameters for kinetics solutions
+    * `thermal hydraulics` parameters come from a thermal hydraulics physics plugin (e.g., flow rates, temperatures, etc.)
     """
 
     depletion = "depletion"
@@ -83,6 +85,8 @@ class Category:
     neutronics = "neutronics"
     gamma = "gamma"
     detailedAxialExpansion = "detailedAxialExpansion"
+    reactivityCoefficients = "reactivity coefficients"
+    thermalHydraulics = "thermal hydraulics"
 
 
 class ParamLocation(enum.Flag):
diff --git a/armi/reactor/reactorParameters.py b/armi/reactor/reactorParameters.py
index 570d5429..2fe1bc5b 100644
--- a/armi/reactor/reactorParameters.py
+++ b/armi/reactor/reactorParameters.py
@@ -629,6 +629,12 @@ def defineCoreParameters():
             description="Fuel Axial Expansion Coefficient",
         )
 
+        pb.defParam(
+            "rxFuelAxialExpansionCoeffPerPercent",
+            units="dk/kk'-%",
+            description="Fuel Axial Expansion Coefficient",
+        )
+
         pb.defParam(
             "rxGridPlateRadialExpansionCoeffPerTemp",
             units=f"{units.REACTIVITY}/{units.DEGK}",
diff --git a/armi/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py
index 16a126ff..89e909e8 100644
--- a/armi/settings/fwSettings/globalSettings.py
+++ b/armi/settings/fwSettings/globalSettings.py
@@ -17,9 +17,6 @@ Framework-wide settings definitions and constants.
 
 This should contain Settings definitions for general-purpose "framework" settings. These
 should only include settings that are not related to any particular physics or plugins.
-
-TODO: There are lots of settings in here that violate the above rule, which still need
-to be migrated to their respective plugins: they are clearly separated for review.
 """
 import os
 from typing import List
diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst
index 545ad762..44065b86 100644
--- a/doc/release/0.2.rst
+++ b/doc/release/0.2.rst
@@ -15,6 +15,7 @@ What's new in ARMI
 #. Add python 3.11 to ARMI's CI testing GH actions! (`PR#1341 <https://github.com/terrapower/armi/pull/1341>`_)
 #. Put back ``avgFuelTemp`` block parameter. (`PR#1362 <https://github.com/terrapower/armi/pull/1362>`_)
 #. Make cylindrical component block collection less strict about pre-homogenization checks. (`PR#1347 <https://github.com/terrapower/armi/pull/1347>`_)
+#. Updated some parameter definitions and defaults. (`PR#1355 <https://github.com/terrapower/armi/pull/1355>`_)
 #. TBD
 
 Bug fixes
diff --git a/ruff.toml b/ruff.toml
index dc9e3223..402d61c6 100644
--- a/ruff.toml
+++ b/ruff.toml
@@ -76,6 +76,7 @@ line-length = 88
 # N - We have our own naming conventions for unit tests.
 # SLF001 - private member access
 "*/tests/*" = ["D1", "E741", "N", "SLF001"]
+"doc/gallery-src/*" = ["D400"]
 
 [pydocstyle]
 # Use numpy-style docstrings.


Test patch:
diff --git a/armi/reactor/converters/tests/test_uniformMesh.py b/armi/reactor/converters/tests/test_uniformMesh.py
index 35fc66a3..fb1ad27a 100644
--- a/armi/reactor/converters/tests/test_uniformMesh.py
+++ b/armi/reactor/converters/tests/test_uniformMesh.py
@@ -415,8 +415,8 @@ class TestUniformMesh(unittest.TestCase):
 
         self.converter.convert(self.r)
         for ib, b in enumerate(self.converter.convReactor.core.getBlocks()):
-            b.p.mgFlux = range(33)
-            b.p.adjMgFlux = range(33)
+            b.p.mgFlux = list(range(33))
+            b.p.adjMgFlux = list(range(33))
             b.p.fastFlux = 2.0
             b.p.flux = 5.0
             b.p.power = 5.0
@@ -507,8 +507,8 @@ class TestGammaUniformMesh(unittest.TestCase):
 
         # set original parameters on pre-mapped core with non-uniform assemblies
         for b in self.r.core.getBlocks():
-            b.p.mgFlux = range(33)
-            b.p.adjMgFlux = range(33)
+            b.p.mgFlux = list(range(33))
+            b.p.adjMgFlux = list(range(33))
             b.p.fastFlux = 2.0
             b.p.flux = 5.0
             b.p.power = 5.0
diff --git a/armi/reactor/tests/test_grids.py b/armi/reactor/tests/test_grids.py
index 79f0543e..a9b602ba 100644
--- a/armi/reactor/tests/test_grids.py
+++ b/armi/reactor/tests/test_grids.py
@@ -188,6 +188,14 @@ class TestGrid(unittest.TestCase):
         self.assertIsInstance(multiLoc, grids.MultiIndexLocation)
         self.assertIn((1, 0, 0), grid._locations)
 
+    def test_ringPosFromIndicesIncorrect(self):
+        """Test the getRingPos fails if there is no armiObect or parent."""
+        grid = grids.Grid(unitSteps=((1.0, 0.0, 0.0), (0.0, 1.0, 0.0), (0.0, 0.0, 1.0)))
+
+        grid.armiObject = None
+        with self.assertRaises(ValueError):
+            grid.getRingPos(((0, 0), (1, 1)))
+
 
 class TestHexGrid(unittest.TestCase):
     """A set of tests for the Hexagonal Grid.
diff --git a/armi/tests/test_apps.py b/armi/tests/test_apps.py
index 779b1eff..a353b1ab 100644
--- a/armi/tests/test_apps.py
+++ b/armi/tests/test_apps.py
@@ -24,6 +24,8 @@ from armi import getDefaultPluginManager
 from armi import isStableReleaseVersion
 from armi import meta
 from armi import plugins
+from armi import utils
+from armi.reactor.flags import Flags
 from armi.__main__ import main
 
 
@@ -119,6 +121,19 @@ class TestApps(unittest.TestCase):
         ):
             app.getParamRenames()
 
+    def test_registerPluginFlags(self):
+        # set up the app, pm, and register some plugins
+        app = getApp()
+
+        # validate our flags have been registered
+        self.assertEqual(Flags.fromString("FUEL"), Flags.FUEL)
+        self.assertEqual(Flags.fromString("PRIMARY"), Flags.PRIMARY)
+
+        # validate we can only register the flags once
+        for _ in range(3):
+            with self.assertRaises(RuntimeError):
+                app.registerPluginFlags()
+
     def test_getParamRenamesInvalids(self):
         # a basic test of the method
         app = getApp()