Repository: terrapower/armi

Issue:
Implement more detailed `__lt__` for component sorting
Components on a block are sorted by their `getBoundingCircleOuterDiameter`.

https://github.com/terrapower/armi/blob/abd1db4adfa0c536f9bf9f08f0f1e8b74b1cd0d7/armi/reactor/components/component.py#L272-L288

Sometimes, two components have the same outer diameter, in which case the sorting done by `__lt__` will depend on the order of comparison. i.e., 

`a < b` evaluates to `False`
`b < a` also evaluates to `False`

As a result, 

`sorted([a, b]) = [a, b]`
while
`sorted([b, a]) = [b, a]`

Ideally, the outcome of `sorted` will not be dependent on the order of the input.

We should fall back to `getCircleInnerDiameter` for comparison when sorting components when `getBoundingCircleOuterDiameter` is equivalent. I pushed an example of what I think the sorting should do here:

https://github.com/terrapower/armi/compare/main...component_sort

Discussion:
@john-science: Love it.

Improved consistency, I support this idea.

Solution patch:
diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py
index fc652d57..b058632f 100644
--- a/armi/reactor/blocks.py
+++ b/armi/reactor/blocks.py
@@ -260,7 +260,7 @@ class Block(composites.Composite):
             componentArea = c.getArea(cold=cold)
             if c.isFuel():
                 fuelComponentArea += componentArea
-            elif c.hasFlags(Flags.SLUG):
+            elif c.hasFlags([Flags.SLUG, Flags.DUMMY]):
                 # this flag designates that this clad/slug combination isn't fuel and shouldn't be
                 # counted in the average
                 pass
diff --git a/armi/reactor/components/__init__.py b/armi/reactor/components/__init__.py
index 8b5a938a..8035e382 100644
--- a/armi/reactor/components/__init__.py
+++ b/armi/reactor/components/__init__.py
@@ -186,6 +186,19 @@ class UnshapedComponent(Component):
         """
         return 2 * math.sqrt(self.getComponentArea(cold=cold) / math.pi)
 
+    def getCircleInnerDiameter(self, Tc=None, cold=False):
+        """
+        Component is unshaped; assume it is circular and there is no ID (return 0.0).
+
+        Parameters
+        ----------
+        Tc : float, optional
+            Ignored for this component
+        cold : bool, optional
+            Ignored for this component
+        """
+        return 0.0
+
     @staticmethod
     def fromComponent(otherComponent):
         """
diff --git a/armi/reactor/components/component.py b/armi/reactor/components/component.py
index cb9a5d86..6630669d 100644
--- a/armi/reactor/components/component.py
+++ b/armi/reactor/components/component.py
@@ -271,19 +271,32 @@ class Component(composites.Composite, metaclass=ComponentType):
         """
         True if a circle encompassing this object has a smaller diameter than one encompassing another component.
 
+        If the bounding circles for both components have identical size, then revert to checking the inner
+        diameter of each component for sorting.
+
         This allows sorting because the Python sort functions only use this method.
         """
         thisOD = self.getBoundingCircleOuterDiameter(cold=True)
         thatOD = other.getBoundingCircleOuterDiameter(cold=True)
         try:
-            return thisOD < thatOD
-        except Exception:
-            raise ValueError(
-                "Components 1 ({} with OD {}) and 2 ({} and OD {}) cannot be ordered because their "
-                "bounding circle outer diameters are not comparable.".format(
-                    self, thisOD, other, thatOD
+            if thisOD == thatOD:
+                thisID = self.getCircleInnerDiameter(cold=True)
+                thatID = other.getCircleInnerDiameter(cold=True)
+                return thisID < thatID
+            else:
+                return thisOD < thatOD
+        except (NotImplementedError, Exception) as e:
+            if isinstance(e, NotImplementedError):
+                raise NotImplementedError(
+                    f"getCircleInnerDiameter not implemented for at least one of {self}, {other}"
+                )
+            else:
+                raise ValueError(
+                    "Components 1 ({} with OD {}) and 2 ({} and OD {}) cannot be ordered because their "
+                    "bounding circle outer diameters are not comparable.".format(
+                        self, thisOD, other, thatOD
+                    )
                 )
-            )
 
     def __setstate__(self, state):
         composites.Composite.__setstate__(self, state)
diff --git a/armi/reactor/components/volumetricShapes.py b/armi/reactor/components/volumetricShapes.py
index 27d2bee5..ab8d96a9 100644
--- a/armi/reactor/components/volumetricShapes.py
+++ b/armi/reactor/components/volumetricShapes.py
@@ -249,7 +249,10 @@ class RadialSegment(ShapedComponent):
         return vol
 
     def getBoundingCircleOuterDiameter(self, Tc=None, cold=False):
-        return self.getDimension("outer_radius", Tc, cold)
+        return 2.0 * self.getDimension("outer_radius", Tc, cold)
+
+    def getCircleInnerDiameter(self, Tc=None, cold=False):
+        return 2.0 * self.getDimension("inner_radius", Tc, cold)
 
 
 class DifferentialRadialSegment(RadialSegment):
diff --git a/doc/release/0.5.rst b/doc/release/0.5.rst
index fd5bf7cb..18c32f69 100644
--- a/doc/release/0.5.rst
+++ b/doc/release/0.5.rst
@@ -10,8 +10,8 @@ New Features
 ------------
 #. Move instead of copy files from TemporaryDirectoryChanger. (`PR#2022 <https://github.com/terrapower/armi/pull/2022>`_)
 #. Creating the ``armi.testing`` module, to share ARMI testing tools. (`PR#2028 <https://github.com/terrapower/armi/pull/2028>`_)
-#. Invoking ``component.material.density()``  does not log an expensive stack trace if the material
-   is a fluid. (`PR#2075 <https://github.com/terrapower/armi/pull/2075>`_)
+#. Using inner diameter for sorting components when outer diameter is identical. (`PR#1882 <https://github.com/terrapower/armi/pull/1882>`_)
+#. Invoking ``component.material.density()`` does not log an expensive stack tracefor fluids. (`PR#2075 <https://github.com/terrapower/armi/pull/2075>`_)
 #. TBD
 
 API Changes


Test patch:
diff --git a/armi/reactor/tests/test_components.py b/armi/reactor/tests/test_components.py
index 8c0c9a43..42180367 100644
--- a/armi/reactor/tests/test_components.py
+++ b/armi/reactor/tests/test_components.py
@@ -507,6 +507,52 @@ class TestDerivedShapeGetArea(unittest.TestCase):
         self.assertAlmostEqual(totalAreaCold, totalAreaHot, delta=1e-10)
 
 
+class TestComponentSort(unittest.TestCase):
+    def setUp(self):
+        self.components = []
+        pinComp = components.Circle(
+            "pin", "UZr", Tinput=273.0, Thot=273.0, od=0.08, mult=169.0
+        )
+        gapComp = components.Circle(
+            "gap", "Sodium", Tinput=273.0, Thot=273.0, id=0.08, od=0.08, mult=169.0
+        )
+        ductComp = components.Hexagon(
+            "duct", "HT9", Tinput=273.0, Thot=273.0, op=2.6, ip=2.0, mult=1.0
+        )
+        cladComp = components.Circle(
+            "clad", "HT9", Tinput=273.0, Thot=273.0, id=0.08, od=0.1, mult=169.0
+        )
+        wireComp = components.Helix(
+            "wire",
+            "HT9",
+            Tinput=273.0,
+            Thot=273.0,
+            axialPitch=10.0,
+            helixDiameter=0.11,
+            od=0.01,
+            mult=169.0,
+        )
+        self.components = [
+            wireComp,
+            cladComp,
+            ductComp,
+            pinComp,
+            gapComp,
+        ]
+
+    def test_sorting(self):
+        """Test that components are sorted as expected."""
+        sortedComps = sorted(self.components)
+        currentMaxOd = 0.0
+        for c in sortedComps:
+            self.assertGreaterEqual(
+                c.getBoundingCircleOuterDiameter(cold=True), currentMaxOd
+            )
+            currentMaxOd = c.getBoundingCircleOuterDiameter(cold=True)
+        self.assertEqual(sortedComps[1].name, "gap")
+        self.assertEqual(sortedComps[2].name, "clad")
+
+
 class TestCircle(TestShapedComponent):
     """Test circle shaped component."""
 
@@ -1632,7 +1678,7 @@ class TestRadialSegment(TestShapedComponent):
 
     def test_getBoundingCircleOuterDiameter(self):
         self.assertEqual(
-            self.component.getBoundingCircleOuterDiameter(cold=True), 170.0
+            self.component.getBoundingCircleOuterDiameter(cold=True), 340.0
         )
 
 
@@ -1679,7 +1725,7 @@ class TestDifferentialRadialSegment(TestShapedComponent):
         self.assertFalse(self.component.THERMAL_EXPANSION_DIMS)
 
     def test_getBoundingCircleOuterDiameter(self):
-        self.assertEqual(self.component.getBoundingCircleOuterDiameter(cold=True), 170)
+        self.assertEqual(self.component.getBoundingCircleOuterDiameter(cold=True), 340)
 
 
 class TestMaterialAdjustments(unittest.TestCase):