Repository: terrapower/armi
Pull request #1859 (2024-09-06 11:15:18)

Use a more robust method for determining number of pins in `HexBlock._rotatePins()`
## What is the change?

Use `block.getNumPins()` instead of counting `Flags.CLAD` components. Round this number up to a number of pins that fits exactly into a regular hex lattice.

## Why is the change being made?

1. Using block.getNumPins(): because counting the multiplicity of `Flags.CLAD` components on a block will not always be equal to the number of pins (e.g., a solid steel reflector slug)
2. Rounding up the number of pins: If a block is missing pin locations from a full hex lattice, we should round up the number of pins. The math for mapping the indices only works if the number of pins fits exactly into a regular hex lattice (19, 37, 61, 91, etc.)

---

## Checklist

<!--
    You (the pull requester) should put an `x` in the boxes below you have completed.

    (If a checkbox doesn't apply to your PR, check it anyway.)
-->

- [x] This PR has only [one purpose or idea](https://terrapower.github.io/armi/developer/tooling.html#one-idea-one-pr).
- [ ] [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`.

mgjarrett: Note that I just realized #1846 will make `rotatePins` private, so if that is merged first then I would have to update the release notes, which currently mark this as being an API change.
drewj-tp: The `justCompute=True` behavior is something that I think could be moved off of `HexBlock.rotatePins`. The place it's used in ARMI at least is just to find post-rotation location of pins to minimize burnup

https://github.com/terrapower/armi/blob/f5267e0cc332ac9c97e8fe1c4e49e28076455399/armi/physics/fuelCycle/hexAssemblyFuelMgmtUtils.py#L105

Which feels like overkill to emulate rotating every single pin in a block just to see where one pin would end up. So I put some of the code in `armi.utils.hexagon` in https://github.com/terrapower/armi/pull/1846/commits/afd452893febe9f46f7c007ddf5a27e0aa381af9.

If there are other needs for `justCompute=True`, let me know and I can try to fold them in to #1846 or we can put them in a new ticket
mgjarrett: Marking this as draft until #1846 is merged; this PR will be updated after that
mgjarrett: after #1846 made `_rotatePins` private, it no longer makes sense to add the optional `numPins` argument to this function, because users won't be calling this function directly anyway.

However, I think it could still be worthwhile to make the determination of the number of pins more robust. `Block` has a `getNumPins` method that is more robust than the one-liner used.

https://github.com/terrapower/armi/blob/abd1db4adfa0c536f9bf9f08f0f1e8b74b1cd0d7/armi/reactor/blocks.py#L1047-L1079

 Is there any reason not to use `getNumPins` here?

https://github.com/terrapower/armi/blob/abd1db4adfa0c536f9bf9f08f0f1e8b74b1cd0d7/armi/reactor/blocks.py#L2141

Patch:
diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py
index bef0ade5..cc18826a 100644
--- a/armi/reactor/blocks.py
+++ b/armi/reactor/blocks.py
@@ -2136,10 +2136,12 @@ class HexBlock(Block):
                 "Cannot rotate {0} to rotNum {1}. Must be 0-5. ".format(self, rotNum)
             )
 
-        # Pin numbers start at 1. Number of pins in the block is assumed to be based on
-        # cladding count.
-        numPins = self.getNumComponents(Flags.CLAD)
-        rotateIndexLookup = dict(zip(range(1, numPins + 1), range(1, numPins + 1)))
+        numPins = self.getNumPins()
+        hexRings = hexagon.numRingsToHoldNumCells(numPins)
+        fullNumPins = hexagon.totalPositionsUpToRing(hexRings)
+        rotateIndexLookup = dict(
+            zip(range(1, fullNumPins + 1), range(1, fullNumPins + 1))
+        )
 
         # Look up the current orientation and add this to it. The math below just rotates
         # from the reference point so we need a total rotation.
@@ -2147,7 +2149,7 @@ class HexBlock(Block):
 
         # non-trivial rotation requested
         # start at 2 because pin 1 never changes (it's in the center!)
-        for pinNum in range(2, numPins + 1):
+        for pinNum in range(2, fullNumPins + 1):
             if rotNum == 0:
                 # Rotation to reference orientation. Pin locations are pin IDs.
                 pass
@@ -2162,7 +2164,7 @@ class HexBlock(Block):
         if not justCompute:
             self.setRotationNum(rotNum)
             self.p["pinLocation"] = [
-                rotateIndexLookup[pinNum] for pinNum in range(1, numPins + 1)
+                rotateIndexLookup[pinNum] for pinNum in range(1, fullNumPins + 1)
             ]
 
         return rotateIndexLookup
diff --git a/armi/reactor/blueprints/reactorBlueprint.py b/armi/reactor/blueprints/reactorBlueprint.py
index eb51ea3d..a1481c36 100644
--- a/armi/reactor/blueprints/reactorBlueprint.py
+++ b/armi/reactor/blueprints/reactorBlueprint.py
@@ -144,7 +144,7 @@ class SystemBlueprint(yamlize.Object):
         """
         from armi.reactor import reactors  # avoid circular import
 
-        runLog.info("Constructing the `{}`".format(self.name))
+        runLog.info(f"Constructing the `{self.name}`")
 
         if geom is not None and self.name == "core":
             gridDesign = geom.toGridBlueprints("core")[0]
@@ -196,9 +196,7 @@ class SystemBlueprint(yamlize.Object):
         return system
 
     def _loadAssemblies(self, cs, container, gridContents, bp):
-        runLog.header(
-            "=========== Adding Assemblies to {} ===========".format(container)
-        )
+        runLog.header(f"=========== Adding Assemblies to {container} ===========")
         badLocations = set()
         for locationInfo, aTypeID in gridContents.items():
             newAssembly = bp.constructAssem(cs, specifier=aTypeID)
@@ -232,9 +230,7 @@ class SystemBlueprint(yamlize.Object):
         # (unless specified on input)
         if not gridDesign.latticeDimensions:
             runLog.info(
-                "Updating spatial grid pitch data for {} geometry".format(
-                    container.geomType
-                )
+                f"Updating spatial grid pitch data for {container.geomType} geometry"
             )
             if container.geomType == geometry.GeomType.HEX:
                 container.spatialGrid.changePitch(container[0][0].getPitch())
@@ -258,26 +254,21 @@ def summarizeMaterialData(container):
         Any Core object with Blocks and Components defined.
     """
     runLog.header(
-        "=========== Summarizing Source of Material Data for {} ===========".format(
-            container
-        )
+        f"=========== Summarizing Source of Material Data for {container} ==========="
     )
     materialNames = set()
     materialData = []
     for c in container.iterComponents():
         if c.material.name in materialNames:
             continue
-        materialData.append((c.material.name, c.material.DATA_SOURCE, False))
+        materialData.append((c.material.name, c.material.DATA_SOURCE))
         materialNames.add(c.material.name)
+
     materialData = sorted(materialData)
     runLog.info(
         tabulate.tabulate(
             data=materialData,
-            headers=[
-                "Material Name",
-                "Source Location",
-                "Property Data was Modified\nfrom the Source?",
-            ],
+            headers=["Material Name", "Source Location"],
             tableFmt="armi",
         )
     )
diff --git a/doc/release/0.4.rst b/doc/release/0.4.rst
index cda3e8ff..973d91cf 100644
--- a/doc/release/0.4.rst
+++ b/doc/release/0.4.rst
@@ -12,6 +12,7 @@ New Features
 #. Removing the ``tabulate`` dependency by ingesting it to ``armi.utils.tabulate``. (`PR#1811 <https://github.com/terrapower/armi/pull/1811>`_)
 #. Adding ``--skip-inspection`` flag to ``CompareCases`` CLI. (`PR#1842 <https://github.com/terrapower/armi/pull/1842>`_)
 #. Allow merging a component with zero area into another component (`PR#1858 <https://github.com/terrapower/armi/pull/1858>`_)
+#. Use ``Block.getNumPins()`` in ``HexBlock._rotatePins()``. (`PR#1859 <https://github.com/terrapower/armi/pull/1859>`_)
 #. Provide utilities for determining location of a rotated object in a hexagonal lattice (``getIndexOfRotatedCell``). (`PR#1846 <https://github.com/terrapower/armi/1846`)
 #. Allow merging a component with zero area into another component. (`PR#1858 <https://github.com/terrapower/armi/pull/1858>`_)
 #. Provide ``Parameter.hasCategory`` for quickly checking if a parameter is defined with a given category. (`PR#1899 <https://github.com/terrapower/armi/pull/1899>`_)
@@ -41,6 +42,7 @@ Bug Fixes
 #. 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>`_)
 #. Material theoretical density is serialized to and read from database. (`PR#1852 <https://github.com/terrapower/armi/pull/1852>`_)
+#. Removed broken and unused column in ``summarizeMaterialData``. (`PR#1925 <https://github.com/terrapower/armi/pull/1925>`_)
 #. TBD
 
 Quality Work


Test patch:
diff --git a/armi/reactor/blueprints/tests/test_reactorBlueprints.py b/armi/reactor/blueprints/tests/test_reactorBlueprints.py
index c50645c5..f7d5f618 100644
--- a/armi/reactor/blueprints/tests/test_reactorBlueprints.py
+++ b/armi/reactor/blueprints/tests/test_reactorBlueprints.py
@@ -122,11 +122,7 @@ class TestReactorBlueprints(unittest.TestCase):
 
     def test_materialDataSummary(self):
         """Test that the material data summary for the core is valid as a printout to the stdout."""
-        expectedMaterialData = [
-            ("Custom", "ARMI", False),
-            ("HT9", "ARMI", False),
-            ("UZr", "ARMI", False),
-        ]
+        expectedMaterialData = [("Custom", "ARMI"), ("HT9", "ARMI"), ("UZr", "ARMI")]
         core, _sfp = self._setupReactor()
         materialData = reactorBlueprint.summarizeMaterialData(core)
         for actual, expected in zip(materialData, expectedMaterialData):