Repository: terrapower/armi
Pull request #1393 (2023-08-17 22:04:33)

Removing unused settings
## What is the change?

I removed the settings:

- `CONF_CONDITIONAL_MODULE_NAME`
- `CONF_MEM_PER_NODE`
- `CONF_NUM_CONTROL_BLOCKS`
- `CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD`

Two of the settings marked under the `TODO` before I found were actually used in multiple downstream repos. so I determined to leave them in ARMI.

## Why is the change being made?

I am just solving an existing `TODO` in the repo.  One of these is only used in a single downstream repo, so I have opened a PR there for that. But the other three I determined are entirely unused anywhere.

---

## Checklist

- [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`.


albeanth: ![more-niice](https://github.com/terrapower/armi/assets/18561046/39421ffb-f2fb-4e8d-ac56-97ad59bb452c)


Patch:
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/reactor/blocks.py b/armi/reactor/blocks.py
index a7a11550..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):
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/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/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py
index 16a126ff..3bf5cfb8 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
@@ -74,6 +71,7 @@ CONF_EXPLICIT_REPEAT_SHUFFLES = "explicitRepeatShuffles"
 CONF_FLUX_RECON = "fluxRecon"  # strange coupling in fuel handlers
 CONF_FRESH_FEED_TYPE = "freshFeedType"
 CONF_GEOM_FILE = "geomFile"
+CONF_GROW_TO_FULL_CORE_AFTER_LOAD = "growToFullCoreAfterLoad"
 CONF_INDEPENDENT_VARIABLES = "independentVariables"
 CONF_INITIALIZE_BURN_CHAIN = "initializeBurnChain"
 CONF_INPUT_HEIGHTS_HOT = "inputHeightsConsideredHot"
@@ -96,6 +94,7 @@ CONF_POWER = "power"
 CONF_POWER_FRACTIONS = "powerFractions"
 CONF_PROFILE = "profile"
 CONF_REALLY_SMALL_RUN = "reallySmallRun"
+CONF_REMOVE_PER_CYCLE = "removePerCycle"
 CONF_RUN_TYPE = "runType"
 CONF_SKIP_CYCLES = "skipCycles"
 CONF_SMALL_RUN = "smallRun"
@@ -117,14 +116,6 @@ CONF_VERBOSITY = "verbosity"
 CONF_VERSIONS = "versions"
 CONF_ZONE_DEFINITIONS = "zoneDefinitions"
 
-# TODO: Unused by ARMI, slated for removal
-CONF_CONDITIONAL_MODULE_NAME = "conditionalModuleName"  # mcfr
-CONF_GROW_TO_FULL_CORE_AFTER_LOAD = "growToFullCoreAfterLoad"  # mcnp & gui
-CONF_MEM_PER_NODE = "memPerNode"  # unused
-CONF_NUM_CONTROL_BLOCKS = "numControlBlocks"  # unused
-CONF_REMOVE_PER_CYCLE = "removePerCycle"  # crucible, equilibrium, gui
-CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD = "useInputTemperaturesOnDBLoad"  # unused
-
 
 def defineSettings() -> List[setting.Setting]:
     """Return a list of global framework settings."""
@@ -200,14 +191,6 @@ def defineSettings() -> List[setting.Setting]:
                 "If false, block heights are at cold/as-built dimensions and will be thermally expanded as appropriate."
             ),
         ),
-        setting.Setting(
-            CONF_CONDITIONAL_MODULE_NAME,
-            default="",
-            label="Burn End Conditional",
-            description="File name (directory not included) of the Python "
-            "module that contains a conditional function to determine the end of burn "
-            "cycles",
-        ),
         setting.Setting(
             CONF_AUTOMATIC_VARIABLE_MESH,
             default=False,
@@ -574,12 +557,6 @@ def defineSettings() -> List[setting.Setting]:
             description="Description needed",
             schema=vol.All(vol.Coerce(float), vol.Range(min=0, max=1)),
         ),
-        setting.Setting(
-            CONF_MEM_PER_NODE,
-            default=2000,
-            label="Memory per Node",
-            description="Memory requested per cluster node",
-        ),
         setting.Setting(
             CONF_MPI_TASKS_PER_NODE,
             default=0,
@@ -599,12 +576,6 @@ def defineSettings() -> List[setting.Setting]:
             "cycles to be run after `startCycle`.",
             schema=vol.All(vol.Coerce(int), vol.Range(min=1)),
         ),
-        setting.Setting(
-            CONF_NUM_CONTROL_BLOCKS,
-            default=6,
-            label="Number of Control Blocks",
-            description="Number of blocks with control for a REBUS poison search",
-        ),
         setting.Setting(
             CONF_TIGHT_COUPLING,
             default=False,
@@ -780,14 +751,6 @@ def defineSettings() -> List[setting.Setting]:
             description="The outlet temperature of the reactor in C",
             schema=vol.All(vol.Coerce(float), vol.Range(min=-273.15)),
         ),
-        setting.Setting(
-            CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD,
-            default=False,
-            label="Temperatures From Input on DB Load",
-            description="When loading from a database, first set all component "
-            "temperatures to the input temperatures. Required when a coupled TH "
-            "case is being derived from a case without any coupled TH.",
-        ),
         setting.Setting(
             CONF_DEFERRED_INTERFACES_CYCLE,
             default=0,


Test patch:
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 a353b1ab..6b534b54 100644
--- a/armi/tests/test_apps.py
+++ b/armi/tests/test_apps.py
@@ -24,7 +24,6 @@ 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