Skip to content

Commit

Permalink
Fixing some miscellaneous TODOs (#1392)
Browse files Browse the repository at this point in the history
  • Loading branch information
john-science authored Aug 17, 2023
1 parent 0f32729 commit 5403ec2
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 27 deletions.
1 change: 0 additions & 1 deletion armi/bookkeeping/db/databaseInterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 2 additions & 9 deletions armi/bookkeeping/db/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions armi/bookkeeping/memoryProfiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion armi/reactor/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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):
Expand Down
1 change: 0 additions & 1 deletion armi/reactor/blueprints/assemblyBlueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions armi/reactor/composites.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 2 additions & 5 deletions armi/reactor/grids.py
Original file line number Diff line number Diff line change
Expand Up @@ -1054,18 +1054,15 @@ 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
and self.armiObject.parent.spatialGrid is not None
):
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."""
Expand Down
8 changes: 8 additions & 0 deletions armi/reactor/tests/test_grids.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions armi/settings/fwSettings/globalSettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5403ec2

Please sign in to comment.