Repository: terrapower/armi
Pull request #1879 (2024-09-14 00:54:34)

Cleanup out defunct to do comments
## What is the change?

The repo has a ton of `TODO` comments in it. I went through the ones in the test files and found most of them were defunct. (Or I just opened a ticket to cover them later.)

## Why is the change being made?

This is the beginning of me finally doing the work to purge ARMI of all `TODO` statements. I will see if they are still relevant. If they are, I will fix them or make them a ticket.

---

## Checklist

- [x] This PR has only [one purpose or idea](https://terrapower.github.io/armi/developer/tooling.html#one-idea-one-pr).
- [x] [Tests](https://terrapower.github.io/armi/developer/tooling.html#test-it) have been added/updated to verify any new/changed code.

<!-- 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/developer/tooling.html#add-release-notes) have been updated if necessary.
- [x] The [documentation](https://terrapower.github.io/armi/developer/tooling.html#document-it) is still up-to-date in the `doc` folder.
- [x] The dependencies are still up-to-date in `pyproject.toml`.

john-science: Not properly merged into main. I'll fix it.

Patch:
diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml
index 7207a57b..6107ecf3 100644
--- a/.github/workflows/coverage.yaml
+++ b/.github/workflows/coverage.yaml
@@ -13,6 +13,10 @@ on:
     paths-ignore:
       - 'doc/**'
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
     # Deploying coverage to coveralls.io should not happen on forks
diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml
index ca1d20cb..a13b6734 100644
--- a/.github/workflows/docs.yaml
+++ b/.github/workflows/docs.yaml
@@ -5,6 +5,10 @@ on:
     branches:
       - main
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
     # Building and deploying docs is broken on forked repos
diff --git a/armi/reactor/composites.py b/armi/reactor/composites.py
index db0e6499..93439c0a 100644
--- a/armi/reactor/composites.py
+++ b/armi/reactor/composites.py
@@ -504,7 +504,6 @@ class ArmiObject(metaclass=CompositeModelType):
         ----------
         other : ArmiObject
             The object to copy params from
-
         """
         self.p = other.p.__class__()
         for p, val in other.p.items():
diff --git a/armi/reactor/converters/axialExpansionChanger/axialExpansionChanger.py b/armi/reactor/converters/axialExpansionChanger/axialExpansionChanger.py
index 2800925b..390305be 100644
--- a/armi/reactor/converters/axialExpansionChanger/axialExpansionChanger.py
+++ b/armi/reactor/converters/axialExpansionChanger/axialExpansionChanger.py
@@ -19,8 +19,8 @@ from armi import runLog
 from armi.reactor.converters.axialExpansionChanger.assemblyAxialLinkage import (
     AssemblyAxialLinkage,
 )
-from armi.reactor.converters.axialExpansionChanger.expansionData import ExpansionData
 from armi.reactor.converters.axialExpansionChanger.expansionData import (
+    ExpansionData,
     getSolidComponents,
 )
 from armi.reactor.flags import Flags
@@ -359,8 +359,8 @@ class AxialExpansionChanger:
 
             _checkBlockHeight(b)
             # Call Component.clearCache to update the Component volume, and therefore the masses,
-            # of all solid components.
-            for c in getSolidComponents(b):
+            # of all components.
+            for c in b:
                 c.clearCache()
             # redo mesh -- functionality based on assembly.calculateZCoords()
             mesh.append(b.p.ztop)
diff --git a/doc/release/0.4.rst b/doc/release/0.4.rst
index 700d90bd..2515ce43 100644
--- a/doc/release/0.4.rst
+++ b/doc/release/0.4.rst
@@ -35,6 +35,7 @@ Bug Fixes
 #. Fixed ``PermissionError`` when using ``syncDbAfterWrite``. (`PR#1857 <https://github.com/terrapower/armi/pull/1857>`_)
 #. Fixed ``MpiDirectoryChanger``. (`PR#1853 <https://github.com/terrapower/armi/pull/1853>`_)
 #. Changed data type of ``thKernel`` setting from ``bool`` to ``str`` in ``ThermalHydraulicsPlugin``. (`PR#1855 <https://github.com/terrapower/armi/pull/1855>`_)
+#. Update height of fluid components after axial expansion (`PR#1828 <https://github.com/terrapower/armi/pull/1828>`_)
 #. TBD
 
 Quality Work


Test patch:
diff --git a/.github/workflows/mac_tests.yaml b/.github/workflows/mac_tests.yaml
index 75d46653..fc2b0f85 100644
--- a/.github/workflows/mac_tests.yaml
+++ b/.github/workflows/mac_tests.yaml
@@ -5,12 +5,18 @@ permissions:
 
 on:
   push:
+    branches:
+      - main
     paths-ignore:
       - 'doc/**'
   pull_request:
     paths-ignore:
       - 'doc/**'
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
 
diff --git a/.github/workflows/unittests.yaml b/.github/workflows/unittests.yaml
index d53b8b4d..a25a1d68 100644
--- a/.github/workflows/unittests.yaml
+++ b/.github/workflows/unittests.yaml
@@ -11,6 +11,10 @@ on:
     paths-ignore:
       - 'doc/**'
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
 
diff --git a/.github/workflows/wintests.yaml b/.github/workflows/wintests.yaml
index 1d77b551..e9131aff 100644
--- a/.github/workflows/wintests.yaml
+++ b/.github/workflows/wintests.yaml
@@ -5,12 +5,18 @@ permissions:
 
 on:
   push:
+    branches:
+      - main
     paths-ignore:
       - 'doc/**'
   pull_request:
     paths-ignore:
       - 'doc/**'
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build:
 
diff --git a/armi/bookkeeping/db/tests/test_comparedb3.py b/armi/bookkeeping/db/tests/test_comparedb3.py
index 0b8ff176..408a69a3 100644
--- a/armi/bookkeeping/db/tests/test_comparedb3.py
+++ b/armi/bookkeeping/db/tests/test_comparedb3.py
@@ -135,7 +135,7 @@ class TestCompareDB3(unittest.TestCase):
             inputFileName="smallestTestReactor/armiRunSmallest.yaml",
         )
 
-        # create two DBs, identical but for file names
+        # create two DBs, identical but for file names and cycle lengths
         dbs = []
         for lenCycle in range(1, 3):
             # build some test data
diff --git a/armi/bookkeeping/db/tests/test_database3.py b/armi/bookkeeping/db/tests/test_database3.py
index d982ed32..1c2530de 100644
--- a/armi/bookkeeping/db/tests/test_database3.py
+++ b/armi/bookkeeping/db/tests/test_database3.py
@@ -315,7 +315,6 @@ class TestDatabase3Smaller(unittest.TestCase):
         with self.assertRaises(KeyError):
             self.db.genAuxiliaryData((-1, -1))
 
-    # TODO: This should be expanded.
     def test_replaceNones(self):
         """Super basic test that we handle Nones correctly in database read/writes."""
         data3 = np.array([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
diff --git a/armi/nuclearDataIO/tests/test_xsLibraries.py b/armi/nuclearDataIO/tests/test_xsLibraries.py
index 33c871dd..324206e9 100644
--- a/armi/nuclearDataIO/tests/test_xsLibraries.py
+++ b/armi/nuclearDataIO/tests/test_xsLibraries.py
@@ -53,8 +53,8 @@ DLAYXS_MCC3 = os.path.join(FIXTURE_DIR_CCCC, "mc2v3.dlayxs")
 UFG_FLUX_EDIT = os.path.join(FIXTURE_DIR, "mc2v3-AA.flux_ufg")
 
 
-class TempFileMixin(unittest.TestCase):
-    """really a test case."""
+class TempFileMixin:
+    """Not a test; just helpful test tooling."""
 
     def setUp(self):
         self.td = TemporaryDirectoryChanger()
@@ -71,7 +71,7 @@ class TempFileMixin(unittest.TestCase):
         )
 
 
-class TestXSLibrary(TempFileMixin):
+class TestXSLibrary(TempFileMixin, unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.isotxsAA = isotxs.readBinary(ISOTXS_AA)
@@ -228,7 +228,7 @@ class TestXSLibrary(TempFileMixin):
     def _canWritefromCombined(self, writer, refFile):
         if self.xsLibGenerationErrorStack is not None:
             print(self.xsLibGenerationErrorStack)
-            raise Exception("see stdout for stack trace")
+            raise Exception("See stdout for stack trace")
         # check to make sure they labels overlap... or are actually the same
         writer.writeBinary(self.xsLib, self.testFileName)
         self.assertTrue(filecmp.cmp(refFile, self.testFileName))
@@ -287,45 +287,23 @@ class TestGetISOTXSFilesInWorkingDirectory(unittest.TestCase):
         self.assertEqual(set(), container & set(shouldNotBeThere))
 
 
-class TestXSlibraryMerging(TempFileMixin):
+class AbstractTestXSlibraryMerging(TempFileMixin):
     """
     A shared class that defines tests that should be true for all IsotxsLibrary merging.
 
     Notes
     -----
-    This is just a base class, so it isn't run directly.
+    This is just a base class, it isn't run directly.
     """
 
-    @classmethod
-    def setUpClass(cls):
-        cls.libAA = None
-        cls.libAB = None
-        cls.libCombined = None
-        cls.libLumped = None
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.libAA = None
-        cls.libAB = None
-        cls.libCombined = None
-        cls.libLumped = None
-        del cls.libAA
-        del cls.libAB
-        del cls.libCombined
-        del cls.libLumped
-
     def setUp(self):
         TempFileMixin.setUp(self)
-        # load a library that is in the ARMI tree. This should
-        # be a small library with LFPs, Actinides, structure, and coolant
-        for attrName, path in [
-            ("libAA", self.getLibAAPath),
-            ("libAB", self.getLibABPath),
-            ("libCombined", self.getLibAA_ABPath),
-            ("libLumped", self.getLibLumpedPath),
-        ]:
-            if getattr(self.__class__, attrName) is None:
-                setattr(self.__class__, attrName, self.getReadFunc()(path()))
+        # Load a library that is in the ARMI tree. This should be a small library with LFPs,
+        # Actinides, structure, and coolant
+        self.libAA = self.getReadFunc()(self.getLibAAPath())
+        self.libAB = self.getReadFunc()(self.getLibABPath())
+        self.libCombined = self.getReadFunc()(self.getLibAA_ABPath())
+        self.libLumped = self.getReadFunc()(self.getLibLumpedPath())
 
     def getErrorType(self):
         raise NotImplementedError()
@@ -368,16 +346,16 @@ class TestXSlibraryMerging(TempFileMixin):
     def test_mergeEmptyXSLibWithOtherEssentiallyClonesTheOther(self):
         emptyXSLib = xsLibraries.IsotxsLibrary()
         emptyXSLib.merge(self.libAA)
-        self.__class__.libAA = None
+        self.libAA = None
         self.getWriteFunc()(emptyXSLib, self.testFileName)
         self.assertTrue(filecmp.cmp(self.getLibAAPath(), self.testFileName))
 
     def test_mergeTwoXSLibFiles(self):
         emptyXSLib = xsLibraries.IsotxsLibrary()
         emptyXSLib.merge(self.libAA)
-        self.__class__.libAA = None
+        self.libAA = None
         emptyXSLib.merge(self.libAB)
-        self.__class__.libAB = None
+        self.libAB = None
         self.assertEqual(
             set(self.libCombined.nuclideLabels), set(emptyXSLib.nuclideLabels)
         )
@@ -388,9 +366,9 @@ class TestXSlibraryMerging(TempFileMixin):
     def test_canRemoveIsotopes(self):
         emptyXSLib = xsLibraries.IsotxsLibrary()
         emptyXSLib.merge(self.libAA)
-        self.__class__.libAA = None
+        self.libAA = None
         emptyXSLib.merge(self.libAB)
-        self.__class__.libAB = None
+        self.libAB = None
         for nucId in [
             "ZR93_7",
             "ZR95_7",
@@ -414,7 +392,7 @@ class TestXSlibraryMerging(TempFileMixin):
         self.assertTrue(filecmp.cmp(self.getLibLumpedPath(), self.testFileName))
 
 
-class Pmatrx_merge_Tests(TestXSlibraryMerging):
+class Pmatrx_Merge_Tests(AbstractTestXSlibraryMerging, unittest.TestCase):
     def getErrorType(self):
         return OSError
 
@@ -448,7 +426,7 @@ class Pmatrx_merge_Tests(TestXSlibraryMerging):
             dummyXsLib.merge(self.libCombined)
 
 
-class Isotxs_merge_Tests(TestXSlibraryMerging):
+class Isotxs_Merge_Tests(AbstractTestXSlibraryMerging, unittest.TestCase):
     def getErrorType(self):
         return OSError
 
@@ -471,7 +449,7 @@ class Isotxs_merge_Tests(TestXSlibraryMerging):
         return ISOTXS_LUMPED
 
 
-class Gamiso_merge_Tests(TestXSlibraryMerging):
+class Gamiso_Merge_Tests(AbstractTestXSlibraryMerging, unittest.TestCase):
     def getErrorType(self):
         return OSError
 
@@ -494,48 +472,17 @@ class Gamiso_merge_Tests(TestXSlibraryMerging):
         return GAMISO_LUMPED
 
 
-class Combined_merge_Tests(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        cls.isotxsAA = None
-        cls.isotxsAB = None
-        cls.gamisoAA = None
-        cls.gamisoAB = None
-        cls.pmatrxAA = None
-        cls.pmatrxAB = None
-        cls.libCombined = None
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.isotxsAA = None
-        cls.isotxsAB = None
-        cls.gamisoAA = None
-        cls.gamisoAB = None
-        cls.pmatrxAA = None
-        cls.pmatrxAB = None
-        cls.libCombined = None
-        del cls.isotxsAA
-        del cls.isotxsAB
-        del cls.gamisoAA
-        del cls.gamisoAB
-        del cls.pmatrxAA
-        del cls.pmatrxAB
-        del cls.libCombined
-
+class Combined_Merge_Tests(unittest.TestCase):
     def setUp(self):
-        # load a library that is in the ARMI tree. This should
-        # be a small library with LFPs, Actinides, structure, and coolant
-        for attrName, path, readFunc in [
-            ("isotxsAA", ISOTXS_AA, isotxs.readBinary),
-            ("gamisoAA", GAMISO_AA, gamiso.readBinary),
-            ("pmatrxAA", PMATRX_AA, pmatrx.readBinary),
-            ("isotxsAB", ISOTXS_AB, isotxs.readBinary),
-            ("gamisoAB", GAMISO_AB, gamiso.readBinary),
-            ("pmatrxAB", PMATRX_AB, pmatrx.readBinary),
-            ("libCombined", ISOTXS_AA_AB, isotxs.readBinary),
-        ]:
-            if getattr(self.__class__, attrName) is None:
-                setattr(self.__class__, attrName, readFunc(path))
+        # Load a library that is in the ARMI tree. This should be a small library with LFPs,
+        # Actinides, structure, and coolant
+        self.isotxsAA = isotxs.readBinary(ISOTXS_AA)
+        self.gamisoAA = gamiso.readBinary(GAMISO_AA)
+        self.pmatrxAA = pmatrx.readBinary(PMATRX_AA)
+        self.isotxsAB = isotxs.readBinary(ISOTXS_AB)
+        self.gamisoAB = gamiso.readBinary(GAMISO_AB)
+        self.pmatrxAB = pmatrx.readBinary(PMATRX_AB)
+        self.libCombined = isotxs.readBinary(ISOTXS_AA_AB)
 
     def test_mergeAllXSLibFiles(self):
         lib = xsLibraries.IsotxsLibrary()
@@ -543,7 +490,3 @@ class Combined_merge_Tests(unittest.TestCase):
             lib, xsLibrarySuffix="", mergeGammaLibs=True, alternateDirectory=FIXTURE_DIR
         )
         self.assertEqual(set(lib.nuclideLabels), set(self.libCombined.nuclideLabels))
-
-
-# Remove the abstract class, so that it does not run (all tests would fail)
-del TestXSlibraryMerging
diff --git a/armi/reactor/blueprints/tests/test_blockBlueprints.py b/armi/reactor/blueprints/tests/test_blockBlueprints.py
index a06964b5..6215bee4 100644
--- a/armi/reactor/blueprints/tests/test_blockBlueprints.py
+++ b/armi/reactor/blueprints/tests/test_blockBlueprints.py
@@ -332,7 +332,6 @@ class TestGriddedBlock(unittest.TestCase):
         self.assertTrue(a1.hasFlags(Flags.FUEL, exact=True))
         self.assertTrue(a2.hasFlags(Flags.FUEL | Flags.TEST, exact=True))
 
-    # TODO: This test passes, but shouldn't.
     def test_densityConsistentWithComponentConstructor(self):
         a1 = self.blueprints.assemDesigns.bySpecifier["IC"].construct(
             self.cs, self.blueprints
diff --git a/armi/reactor/blueprints/tests/test_blueprints.py b/armi/reactor/blueprints/tests/test_blueprints.py
index 59681637..d63c1246 100644
--- a/armi/reactor/blueprints/tests/test_blueprints.py
+++ b/armi/reactor/blueprints/tests/test_blueprints.py
@@ -35,19 +35,10 @@ from armi.reactor.blueprints.gridBlueprint import saveToStream
 
 
 class TestBlueprints(unittest.TestCase):
-    """Test that the basic functionality of faithfully receiving user input to construct
-    ARMI data model objects works as expected.
+    """Test that the basic functionality of faithfully receiving user input to construct ARMI data
+    model objects works as expected.
 
-    Values are hopefully not hardcoded in here, just sanity checks that nothing messed
-    up as this is code has VERY high incidental coverage from other tests.
-
-    NOTE: as it stands it seems a little hard to test more granularity with the
-    blueprints file as each initialization is intended to be a complete load from the
-    input file, and each load also
-    makes calls out to the reactor for some assembly initialization steps.
-
-    TODO: see the above note, and try to test blueprints on a wider range of input
-    files, touching on each failure case.
+    Try to ensure you test for ideas and not exact matches here, to make the tests more robust.
     """
 
     @classmethod
diff --git a/armi/reactor/converters/tests/test_axialExpansionChanger.py b/armi/reactor/converters/tests/test_axialExpansionChanger.py
index 9906952e..bf69ac64 100644
--- a/armi/reactor/converters/tests/test_axialExpansionChanger.py
+++ b/armi/reactor/converters/tests/test_axialExpansionChanger.py
@@ -451,6 +451,10 @@ class TestConservation(AxialExpansionTestBase, unittest.TestCase):
         .. test:: Ensure the ACLP does not move during fuel-only expansion.
             :id: T_ARMI_AXIAL_EXP_PRESC1
             :tests: R_ARMI_AXIAL_EXP_PRESC
+
+        .. test:: Ensure the component volumes are correctly updated during prescribed expansion.
+            :id: T_ARMI_AXIAL_EXP_PRESC2
+            :tests: R_ARMI_AXIAL_EXP_PRESC
         """
         # build test assembly with ACLP
         assembly = HexAssembly("testAssemblyType")
@@ -473,6 +477,9 @@ class TestConservation(AxialExpansionTestBase, unittest.TestCase):
         aclpZTop = aclp.p.ztop
         aclpZBottom = aclp.p.zbottom
 
+        # get total assembly fluid mass pre-expansion
+        preExpAssemFluidMass = self._getTotalAssemblyFluidMass(assembly)
+
         # expand fuel
         # get fuel components
         cList = [c for b in assembly for c in b if c.hasFlags(Flags.FUEL)]
@@ -481,6 +488,9 @@ class TestConservation(AxialExpansionTestBase, unittest.TestCase):
         chngr = AxialExpansionChanger()
         chngr.performPrescribedAxialExpansion(assembly, cList, pList, setFuel=True)
 
+        # get total assembly fluid mass post-expansion
+        postExpAssemFluidMass = self._getTotalAssemblyFluidMass(assembly)
+
         # do assertion
         self.assertEqual(
             aclpZBottom,
@@ -493,6 +503,26 @@ class TestConservation(AxialExpansionTestBase, unittest.TestCase):
             msg="ACLP ztop has changed. It should not with fuel component only expansion!",
         )
 
+        # verify that the component volumes are correctly updated
+        for b in assembly:
+            for c in b:
+                self.assertAlmostEqual(
+                    c.getArea() * b.getHeight(),
+                    c.getVolume(),
+                    places=12,
+                )
+        # verify that the total assembly fluid mass is preserved through expansion
+        self.assertAlmostEqual(preExpAssemFluidMass, postExpAssemFluidMass, places=11)
+
+    @staticmethod
+    def _getTotalAssemblyFluidMass(assembly) -> float:
+        totalAssemblyFluidMass = 0.0
+        for b in assembly:
+            for c in b:
+                if isinstance(c.material, materials.material.Fluid):
+                    totalAssemblyFluidMass += c.getMass()
+        return totalAssemblyFluidMass
+
     def test_reset(self):
         self.obj.setAssembly(self.a)
         self.obj.reset()
diff --git a/armi/reactor/tests/test_blocks.py b/armi/reactor/tests/test_blocks.py
index 0645a76c..08f729e9 100644
--- a/armi/reactor/tests/test_blocks.py
+++ b/armi/reactor/tests/test_blocks.py
@@ -70,8 +70,6 @@ def buildSimpleFuelBlock():
     b.add(coolant)
     b.add(intercoolant)
 
-    b.getVolumeFractions()  # TODO: remove, should be no-op when removed self.cached
-
     return b
 
 
@@ -232,8 +230,6 @@ def loadTestBlock(cold=True):
     block.add(duct)
     block.add(interSodium)
 
-    block.getVolumeFractions()  # TODO: remove, should be no-op when removed self.cached
-
     block.setHeight(16.0)
 
     block.autoCreateSpatialGrids()