Repository: terrapower/armi
Pull request #2069 (2025-01-31 16:26:32)

Expand allowable XS types to include lower case letters
## What is the change?

<!-- MANDATORY: Describe the change -->

Allow lower case letters to be used as the XS type to increase the number of possible unique XS groups from 26 to 52.

## Why is the change being made?

Some models have a large number of different XS types (15-20), and some internal plugins generate additional XS groups while running, which can push the total number of XS groups above 26. The ARMI app will currently crash if a 27th XS type is requested.

Notes
-------

This should work fine on Linux or Mac platforms, which have case-sensitive filesystems. A user might encounter problems if they attempt to exceed 26 XS types on Windows, because the filesystem is not case-sensitive, and a plugin might try to generate a file name that is indistinguishable to a previous filename on Windows (e.g., `latticePhysics-AA.inp` and then `latticePhysics-aA.inp`). While this is unfortunate, the current code would also fail when a user tries to exceed 26 XS types (on any platform), so this PR isn't breaking a currently working capability, it's just changing the manner in which the code would fail.

<!-- MANDATORY: Explain why the change is necessary -->
<!-- Optional: Link to any related GitHub Issues -->


---

## 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).
- [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 -->

- [ ] 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: Well, the change is certainly easy to understand.

It is unfortunate this wouldn't work on Windows.  (Ridiculous file system.)

@mgjarrett Have you ran into a need for this PR in practice?
drewj-tp: >This should work fine on Linux or Mac platforms, which have case-sensitive filesystems

I would try to find a Mac user and confirm as appropriate. I remember 2022-2023 having some case insensitive file system problems.

Looking for confirmation, it looks like it's possible for a Mac to have a case-insensitive file system

> Choose one of the following APFS formats for Mac computers using macOS 10.13 or later.
> 
> APFS: Uses the APFS format. Choose this option if you don’t need an encrypted or case-sensitive format.
> 
> APFS (Encrypted): Uses the APFS format and encrypts the volume.
> 
> APFS (Case-sensitive): Uses the APFS format and is case-sensitive to file and folder names. For example, folders named “Homework” and “HOMEWORK” are two different folders.
> 
> APFS (Case-sensitive, Encrypted): Uses the APFS format, is case-sensitive to file and folder names, and encrypts the volume. For example, folders named “Homework” and “HOMEWORK” are two different folders.


https://support.apple.com/guide/disk-utility/file-system-formats-dsku19ed921c/mac
mgjarrett: > Well, the change is certainly easy to understand.
> 
> It is unfortunate this wouldn't work on Windows. (Ridiculous file system.)
> 
> @mgjarrett Have you ran into a need for this PR in practice?

Yes, the motivation for the PR is that I encountered this error in a practical application.
john-science: > I would try to find a Mac user and confirm as appropriate.

@drewj-tp Is the unit test here sufficient for you? We run all the ARMI unit tests on Mac, Linux, and Windows on GitHub.
drewj-tp: > > I would try to find a Mac user and confirm as appropriate.
> 
> @drewj-tp Is the unit test here sufficient for you? We run all the ARMI unit tests on Mac, Linux, and Windows on GitHub.

I don't have any issues with it. I just wanted to point out that not all Macs will have case-sensitive file systems. Our GitHub actions runner might so that's okay.

It almost seems like we're putting this as a requirement on the system that uses ARMI. If you don't have a case-sensitive file system, you would either have lots of fatal errors or subtle problems
john-science: Okay, we think this PR needs a docstring explanation of the OS differences. And more importantly a logging statement based on an >26 check, if you're on Windows.
drewj-tp: > Okay, we think this PR needs a docstring explanation of the OS differences

I think something in the user manual (non API docs) is appropriate. Like when we introduce the concept of `xs types`

Patch:
diff --git a/armi/physics/neutronics/crossSectionGroupManager.py b/armi/physics/neutronics/crossSectionGroupManager.py
index f049890b..d7a8831d 100644
--- a/armi/physics/neutronics/crossSectionGroupManager.py
+++ b/armi/physics/neutronics/crossSectionGroupManager.py
@@ -55,6 +55,7 @@ import collections
 import copy
 import os
 import string
+import sys
 
 import numpy as np
 
@@ -80,7 +81,7 @@ def describeInterfaces(cs):
     return None
 
 
-_ALLOWABLE_XS_TYPE_LIST = list(string.ascii_uppercase)
+_ALLOWABLE_XS_TYPE_LIST = list(string.ascii_uppercase + string.ascii_lowercase)
 
 
 def getXSTypeNumberFromLabel(xsTypeLabel: str) -> int:
@@ -1465,6 +1466,17 @@ class CrossSectionGroupManager(interfaces.Interface):
                     len(allocatedXSTypes), len(availableXsTypes), howMany
                 )
             )
+
+        # check for lower-case on case-insensitive file system
+        if sys.platform.startswith("win"):
+            allXSTypes = allocatedXSTypes.union(set(availableXsTypes[:howMany]))
+            allCaps = {c.capitalize() for c in allXSTypes}
+            if len(allCaps) != len(allXSTypes):
+                runLog.warning(
+                    "Mixing upper and lower-case XS group types on a Windows system, which is not "
+                    "case-sensitive. There is a chance that ARMI could overwrite previously "
+                    "generated XS files, which could cause mysterious and/or unpredictable errors."
+                )
         return availableXsTypes[:howMany]
 
     def _getMissingBlueprintBlocks(self, blockCollectionsByXsGroup):
diff --git a/armi/reactor/blueprints/reactorBlueprint.py b/armi/reactor/blueprints/reactorBlueprint.py
index 0af2d81f..e73032be 100644
--- a/armi/reactor/blueprints/reactorBlueprint.py
+++ b/armi/reactor/blueprints/reactorBlueprint.py
@@ -146,6 +146,8 @@ class SystemBlueprint(yamlize.Object):
 
         if geom is not None and self.name == "core":
             gridDesign = geom.toGridBlueprints("core")[0]
+        elif geom is not None and self.name == "Spent Fuel Pool":
+            gridDesign = geom.toGridBlueprints("Spent Fuel Pool")[0]
         else:
             if not bp.gridDesigns:
                 raise ValueError(
diff --git a/armi/reactor/reactors.py b/armi/reactor/reactors.py
index 4354fe60..1a436841 100644
--- a/armi/reactor/reactors.py
+++ b/armi/reactor/reactors.py
@@ -195,7 +195,9 @@ def factory(cs, bp, geom: Optional[SystemLayoutInput] = None) -> Reactor:
             )
 
         for structure in bp.systemDesigns:
-            bpGeom = geom if structure.name.lower() == "core" else None
+            bpGeom = (
+                geom if structure.name.lower() in ("core", "spent fuel pool") else None
+            )
             structure.construct(cs, bp, r, geom=bpGeom)
 
     runLog.debug(f"Reactor: {r}")
diff --git a/armi/reactor/spentFuelPool.py b/armi/reactor/spentFuelPool.py
index 666c646b..463944a2 100644
--- a/armi/reactor/spentFuelPool.py
+++ b/armi/reactor/spentFuelPool.py
@@ -85,8 +85,8 @@ class SpentFuelPool(ExcoreStructure):
 
     def _updateNumberOfColumns(self):
         """Determine the number of columns in the spatial grid."""
-        _locs = self.spatialGrid.items()
-        self.numColumns = len(set([ll[0][0] for ll in _locs]))
+        locs = self.spatialGrid.items()
+        self.numColumns = len(set([ll[0][0] for ll in locs]))
 
     def _getNextLocation(self):
         """Helper method to allow each discharged assembly to be easily dropped into the SFP.
diff --git a/doc/release/0.5.rst b/doc/release/0.5.rst
index 2514a419..fd5bf7cb 100644
--- a/doc/release/0.5.rst
+++ b/doc/release/0.5.rst
@@ -23,6 +23,7 @@ Bug Fixes
 ---------
 #. Fixing check for jagged arrays during ``_writeParams``. (`PR#2051 <https://github.com/terrapower/armi/pull/2051>`_)
 #. Fixing BP-section ignoring tool in ``PassiveDBLoadPlugin``. (`PR#2055 <https://github.com/terrapower/armi/pull/2055>`_)
+#. Making sure SFPs have spatial grids. (`PR#2082 <https://github.com/terrapower/armi/pull/2082>`_)
 #. Fixing number densities when custom isotopics are combined with Fluid components. (`PR#2071 <https://github.com/terrapower/armi/pull/2071>`_)
 #. Fixing scaling of volume-integrated parameters on edge assemblies. (`PR#2060 <https://github.com/terrapower/armi/pull/2060>`_)
 #. Fixing strictness of ``HexGrid`` rough equality check. (`PR#2058 <https://github.com/terrapower/armi/pull/2058>`_)
diff --git a/doc/user/inputs.rst b/doc/user/inputs.rst
index 72891ed8..28c3985e 100644
--- a/doc/user/inputs.rst
+++ b/doc/user/inputs.rst
@@ -788,9 +788,13 @@ xs types
   best to set blocks that have much different compositions to have separate cross section types. The
   tradeoff is that the more XS types you define, the more CPU time the case will take to run.
 
-  Representing xsType by a single letter (A-Z) or number (0-9) limits users to 36 groups. So ARMI
+  Representing xsType by a single capital letter (A-Z) or number (0-9) limits users to 36 groups. So ARMI
   will allow 2-letter xsType designations if and only if the ``buGroups`` setting has length 1 (i.e. no burnup groups are defined). This is useful for high-fidelity XS modeling.
 
+  ARMI is able to use lower-case letters (a-z) for an additional 26 cross section groups, but this 
+  should only be done when working on a case-sensitive file system. On a case-insensitive file system 
+  (Windows, and some MacOS systems) this could cause unpredictable errors.
+
 axial mesh points
   Blocks will be broken up into this many uniform mesh points in the deterministic neutronics
   solvers (e.g. DIF3D). This allows you to define large blocks that have multiple flux points within


Test patch:
diff --git a/armi/physics/neutronics/tests/test_crossSectionManager.py b/armi/physics/neutronics/tests/test_crossSectionManager.py
index 219d852f..4175c5f4 100644
--- a/armi/physics/neutronics/tests/test_crossSectionManager.py
+++ b/armi/physics/neutronics/tests/test_crossSectionManager.py
@@ -20,6 +20,7 @@ Test the cross section manager.
 import copy
 import os
 import pickle
+import sys
 import unittest
 from io import BytesIO
 from unittest.mock import MagicMock
@@ -868,6 +869,19 @@ class TestCrossSectionGroupManager(unittest.TestCase):
         self.assertEqual("C", xsType2)
         self.assertEqual("D", xsType3)
 
+        # verify that we can get lowercase letters
+        xsTypes = self.csm.getNextAvailableXsTypes(27)
+        self.assertEqual("Y", xsTypes[-4])
+        self.assertEqual("a", xsTypes[-3])
+        self.assertEqual("b", xsTypes[-2])
+        self.assertEqual("c", xsTypes[-1])
+
+        # verify that we can get lowercase letters
+        if sys.platform.startswith("win"):
+            with mockRunLogs.BufferLog() as mock:
+                xsTypes = self.csm.getNextAvailableXsTypes(27)
+                self.assertIn("Mixing upper and lower-case XS", mock.getStdout())
+
     def test_getRepresentativeBlocks(self):
         """Test that we can create the representative blocks for a reactor.
 
diff --git a/armi/reactor/tests/test_blocks.py b/armi/reactor/tests/test_blocks.py
index 214bf322..4427b79a 100644
--- a/armi/reactor/tests/test_blocks.py
+++ b/armi/reactor/tests/test_blocks.py
@@ -335,6 +335,34 @@ class TestDetailedNDensUpdate(unittest.TestCase):
         self.assertEqual(block.p.detailedNDens, np.array([0.5]))
 
 
+class TestValidateSFPSpatialGrids(unittest.TestCase):
+    def test_noSFPExists(self):
+        """Validate the spatial grid for a new SFP is None if it was not provided."""
+        # copy the inputs, so we can modify them
+        with TemporaryDirectoryChanger() as newDir:
+            oldDir = os.path.join(TEST_ROOT, "smallestTestReactor")
+            newDir2 = os.path.join(newDir.destination, "smallestTestReactor")
+            shutil.copytree(oldDir, newDir2)
+
+            # cut out the SFP grid in the input file
+            testFile = os.path.join(newDir2, "refSmallestReactor.yaml")
+            txt = open(testFile, "r").read()
+            txt = txt.split("symmetry: full")[0]
+            open(testFile, "w").write(txt)
+
+            # verify there is no spatial grid defined
+            _o, r = loadTestReactor(newDir2, inputFileName="armiRunSmallest.yaml")
+            self.assertIsNone(r.excore.sfp.spatialGrid)
+
+    def test_SFPSpatialGridExists(self):
+        """Validate the spatial grid for a new SFP is not None if it was provided."""
+        _o, r = loadTestReactor(
+            os.path.join(TEST_ROOT, "smallestTestReactor"),
+            inputFileName="armiRunSmallest.yaml",
+        )
+        self.assertIsNotNone(r.excore.sfp.spatialGrid)
+
+
 class Block_TestCase(unittest.TestCase):
     def setUp(self):
         self.block = loadTestBlock()
diff --git a/armi/tests/smallestTestReactor/refSmallestReactor.yaml b/armi/tests/smallestTestReactor/refSmallestReactor.yaml
index 8b9396b5..b539ffd3 100644
--- a/armi/tests/smallestTestReactor/refSmallestReactor.yaml
+++ b/armi/tests/smallestTestReactor/refSmallestReactor.yaml
@@ -18,4 +18,10 @@ grids:
       geom: hex_corners_up
       lattice map: |
         IC
-      symmetry: full
\ No newline at end of file
+      symmetry: full
+    sfp:
+      geom: cartesian
+      symmetry: full
+      lattice pitch:
+        x: 32.0
+        y: 32.0