From 5403ec2d854af27d28477d33bef46535cc7a5f89 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:19:12 -0700 Subject: [PATCH] Fixing some miscellaneous TODOs (#1392) --- armi/bookkeeping/db/databaseInterface.py | 1 - armi/bookkeeping/db/layout.py | 11 ++--------- armi/bookkeeping/memoryProfiler.py | 6 +----- armi/reactor/blocks.py | 6 +++++- armi/reactor/blueprints/assemblyBlueprint.py | 1 - armi/reactor/composites.py | 3 +-- armi/reactor/grids.py | 7 ++----- armi/reactor/tests/test_grids.py | 8 ++++++++ armi/settings/fwSettings/globalSettings.py | 3 --- 9 files changed, 19 insertions(+), 27 deletions(-) diff --git a/armi/bookkeeping/db/databaseInterface.py b/armi/bookkeeping/db/databaseInterface.py index 8fb4e2881..d49f04aec 100644 --- a/armi/bookkeeping/db/databaseInterface.py +++ b/armi/bookkeeping/db/databaseInterface.py @@ -281,7 +281,6 @@ def _checkThatCyclesHistoriesAreEquivalentUpToRestartTime( "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 199c47250..c5bb92498 100644 --- a/armi/bookkeeping/db/layout.py +++ b/armi/bookkeeping/db/layout.py @@ -356,7 +356,7 @@ def _initComps(self, caseTitle, bp): 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 5997249f2..6a1053fe2 100644 --- a/armi/bookkeeping/memoryProfiler.py +++ b/armi/bookkeeping/memoryProfiler.py @@ -307,11 +307,7 @@ def add(self, item): 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 a7a115501..1b2322766 100644 --- a/armi/reactor/blocks.py +++ b/armi/reactor/blocks.py @@ -562,6 +562,8 @@ def adjustUEnrich(self, newEnrich): 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 @@ def completeInitialLoading(self, bolBlock=None): 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 @@ def completeInitialLoading(self, bolBlock=None): self.p.smearDensity = self.getSmearDensity() except ValueError: pass + self.p.enrichmentBOL = self.getFissileMassEnrich() massHmBOL = 0.0 sf = self.getSymmetryFactor() @@ -820,7 +822,9 @@ def completeInitialLoading(self, bolBlock=None): # 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 b0d80b930..727437cea 100644 --- a/armi/reactor/blueprints/assemblyBlueprint.py +++ b/armi/reactor/blueprints/assemblyBlueprint.py @@ -214,7 +214,6 @@ def _createBlock(self, cs, blueprint, bDesign, axialIndex): 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 1c3c1abd9..92b5ad297 100644 --- a/armi/reactor/composites.py +++ b/armi/reactor/composites.py @@ -392,8 +392,7 @@ def __getstate__(self): 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 9ce755450..6c9d7c1ce 100644 --- a/armi/reactor/grids.py +++ b/armi/reactor/grids.py @@ -1054,8 +1054,7 @@ def getRingPos(self, indices) -> Tuple[int, int]: 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 @@ def getRingPos(self, indices) -> Tuple[int, int]: ): 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/tests/test_grids.py b/armi/reactor/tests/test_grids.py index 79f0543e8..a9b602ba5 100644 --- a/armi/reactor/tests/test_grids.py +++ b/armi/reactor/tests/test_grids.py @@ -188,6 +188,14 @@ def test_getitem(self): 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/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py index 16a126ffb..89e909e8e 100644 --- a/armi/settings/fwSettings/globalSettings.py +++ b/armi/settings/fwSettings/globalSettings.py @@ -17,9 +17,6 @@ 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