Repository: terrapower/armi

Issue:
`insertCoreAndAssemblyMaps()` is broken after #1649
I believe that #1649 broke the plotting functionality in [`insertCoreAndAssemblyMaps()`](https://github.com/terrapower/armi/blob/abd1db4adfa0c536f9bf9f08f0f1e8b74b1cd0d7/armi/bookkeeping/report/newReportUtils.py#L617). Specifically, the rotation of assembly patches is not in line with the rotation of the core. Here is an example for the initial core of FFTF (public information):

Before #1649 :
![fftfmc_P1P0_MC2v3_20cm_666dRadialCoreMap](https://github.com/user-attachments/assets/f8b988c0-758b-4140-90d2-f5604240989f)

After #1649 :
![fftfmc_P1P0_MC2v3_20cm_666dRadialCoreMap](https://github.com/user-attachments/assets/7380611f-e2c2-4f4e-b68b-583ecddf1c9c)


Discussion:
keckler: I am currently generating some plots to confirm that this is working.
keckler: Here is a correct corners-up core map that was generated with this branch:
![image](https://github.com/user-attachments/assets/4574fbbf-217d-45f7-bc96-5c7875d70fd0)

Compare that against the pictures that were in the original issue: #1878 .

keckler: Here is the anl-acfi-177 facemap from [our publicly available blueprints](https://terrapower.github.io/armi/tutorials/walkthrough_inputs.html), which is flats-up:
![image](https://github.com/user-attachments/assets/50337000-2303-4b61-8998-0ef62980b289)

So that confirms that flats-up are still working as expected.

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/utils/plotting.py b/armi/utils/plotting.py
index c640cdcf..8e713eeb 100644
--- a/armi/utils/plotting.py
+++ b/armi/utils/plotting.py
@@ -425,7 +425,7 @@ def close(fig=None):
 
 
 def _makeAssemPatches(core):
-    """Return a list of assembly shaped patch for each assembly."""
+    """Return a list of assembly shaped patches for each assembly."""
     patches = []
 
     if isinstance(core.spatialGrid, grids.HexGrid):
@@ -441,8 +441,12 @@ def _makeAssemPatches(core):
     for a in core:
         x, y, _ = a.spatialLocator.getLocalCoordinates()
         if nSides == 6:
+            if core.spatialGrid.cornersUp:
+                orientation = 0
+            else:
+                orientation = math.pi / 2.0
             assemPatch = matplotlib.patches.RegularPolygon(
-                (x, y), nSides, radius=pitch / math.sqrt(3), orientation=math.pi / 2.0
+                (x, y), nSides, radius=pitch / math.sqrt(3), orientation=orientation
             )
         elif nSides == 4:
             # for rectangle x, y is defined as sides instead of center
@@ -942,10 +946,6 @@ def plotBlockFlux(core, fName=None, bList=None, peak=False, adjoint=False, bList
     bList2 :
         a separate list of blocks that will also be plotted on a separate axis on the same plot.
         This is useful for comparing flux in some blocks with flux in some other blocks.
-
-    Notes
-    -----
-    This is not a great method. It should be cleand up and migrated into ``utils.plotting``.
     """
 
     class BlockListFlux:
diff --git a/doc/release/0.4.rst b/doc/release/0.4.rst
index 2515ce43..32244a4f 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>`_)
+#. Rotate hexagonal assembly patches correctly on facemap plots. (`PR#1883 <https://github.com/terrapower/armi/pull/1883>`_)
 #. Update height of fluid components after axial expansion (`PR#1828 <https://github.com/terrapower/armi/pull/1828>`_)
 #. TBD
 


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/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()
diff --git a/armi/tests/smallestTestReactor/refSmallestReactor.yaml b/armi/tests/smallestTestReactor/refSmallestReactor.yaml
index 83f05f7f..8b9396b5 100644
--- a/armi/tests/smallestTestReactor/refSmallestReactor.yaml
+++ b/armi/tests/smallestTestReactor/refSmallestReactor.yaml
@@ -15,7 +15,7 @@ systems:
             z: 6000.0
 grids:
     core:
-      geom: hex
+      geom: hex_corners_up
       lattice map: |
         IC
       symmetry: full
\ No newline at end of file
diff --git a/armi/utils/tests/test_plotting.py b/armi/utils/tests/test_plotting.py
index f4a7b4e9..0fdb9e1c 100644
--- a/armi/utils/tests/test_plotting.py
+++ b/armi/utils/tests/test_plotting.py
@@ -16,6 +16,8 @@
 import os
 import unittest
 
+import numpy as np
+
 from armi.nuclearDataIO.cccc import isotxs
 from armi.reactor.flags import Flags
 from armi.reactor.tests import test_reactors
@@ -127,3 +129,57 @@ class TestPlotting(unittest.TestCase):
 
     def _checkExists(self, fName):
         self.assertTrue(os.path.exists(fName))
+
+
+class TestPatches(unittest.TestCase):
+    """Test the ability to correctly make patches."""
+
+    def test_makeAssemPatches(self):
+        # this one is flats-up with many assemblies in the core
+        _, rHexFlatsUp = test_reactors.loadTestReactor()
+
+        nAssems = len(rHexFlatsUp.core)
+        self.assertGreater(nAssems, 1)
+        patches = plotting._makeAssemPatches(rHexFlatsUp.core)
+        self.assertEqual(len(patches), nAssems)
+
+        # find the patch corresponding to the center assembly
+        for patch in patches:
+            if np.allclose(patch.xy, (0, 0)):
+                break
+
+        vertices = patch.get_verts()
+        # there should be 1 more than the number of points in the shape
+        self.assertEqual(len(vertices), 7)
+        # for flats-up, the first vertex should have a y position of ~zero
+        self.assertAlmostEqual(vertices[0][1], 0)
+
+        # this one is corners-up, with only a single assembly
+        _, rHexCornersUp = test_reactors.loadTestReactor(
+            inputFileName="smallestTestReactor/armiRunSmallest.yaml"
+        )
+
+        nAssems = len(rHexCornersUp.core)
+        self.assertEqual(nAssems, 1)
+        patches = plotting._makeAssemPatches(rHexCornersUp.core)
+        self.assertEqual(len(patches), 1)
+
+        vertices = patches[0].get_verts()
+        self.assertEqual(len(vertices), 7)
+        # for corners-up, the first vertex should have an x position of ~zero
+        self.assertAlmostEqual(vertices[0][0], 0)
+
+        # this one is cartestian, with many assemblies in the core
+        _, rCartesian = test_reactors.loadTestReactor(
+            inputFileName="refTestCartesian.yaml"
+        )
+
+        nAssems = len(rCartesian.core)
+        self.assertGreater(nAssems, 1)
+        patches = plotting._makeAssemPatches(rCartesian.core)
+        self.assertEqual(nAssems, len(patches))
+
+        # just pick a given patch and ensure that it is square-like. orientation
+        # is not important here.
+        vertices = patches[0].get_verts()
+        self.assertEqual(len(vertices), 5)