Repository: terrapower/armi
Pull request #1788 (2024-07-19 16:55:57)

Fix MultipleComponentMerger with DerivedShape solvent
## What is the change?

Simple bug fix for `MultipleComponentMerger`.

Add a check to avoid attempting to run `_verifyExpansion` with a `DerivedShape` solvent. `ComponentMerger` bypasses this check (which doesn't make sense for a `DerivedShape` solvent) here:

https://github.com/terrapower/armi/blob/ed35e229b2b2f5fef1a4b5dd0801d811cae31e31/armi/reactor/converters/blockConverters.py#L97-L98

`MultipleComponentMerger` waits to verify expansion until all components are merged, but the direct call to `_verifyExpansion` avoids the `DerivedShape` component check. I manually added the check to the `_verifyExpansion` call in `MultipleComponentMerger`.

## Why is the change being made?

Currently, `MultipleComponentMerger` fails when solvent is a `DerivedShape` `Component`. Reproducible example of the bug:

```python
import armi
armi.configure()

# Using the FFTF case
o = armi.init(fName='FFTF.yaml')
# Get a block with coolant and wire
b = o.r.core.getBlocks()[200]
# ComponentMerger works fine
newb = armi.reactor.converters.blockConverters.ComponentMerger(b, "wire", "coolant").convert()
# MultipleComponentMerger breaks
newb = armi.reactor.converters.blockConverters.MultipleComponentMerger(b, ["wire"], "coolant").convert()
```

---

## 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`.

AidanMcDonald: @john-science , This is equivalent to the bug fix that I submitted before and then said was unnecessary. Due to some poor git management on my part, I still had the fix when I said it was unnecessary, sorry for the confusion.

Patch:
diff --git a/armi/bookkeeping/db/database3.py b/armi/bookkeeping/db/database3.py
index 52da6aae..99e3efdd 100644
--- a/armi/bookkeeping/db/database3.py
+++ b/armi/bookkeeping/db/database3.py
@@ -1523,7 +1523,6 @@ def packSpecialData(
     composites that are storing the parameters. For instance, if we are dealing with a
     Block parameter, the first index in the numpy array of data is the block index; so
     if each block has a parameter that is a dictionary, ``data`` would be a ndarray,
-                    offset += arr.size
     where each element is a dictionary. This routine supports a number of different
     "strange" things:
 
diff --git a/armi/bookkeeping/memoryProfiler.py b/armi/bookkeeping/memoryProfiler.py
index cc22658a..2ca8752c 100644
--- a/armi/bookkeeping/memoryProfiler.py
+++ b/armi/bookkeeping/memoryProfiler.py
@@ -340,14 +340,19 @@ class SystemAndProcessMemoryUsage:
         # directly by the standard operator and reports, so easier said than done.
         self.percentNodeRamUsed: Optional[float] = None
         self.processMemoryInMB: Optional[float] = None
+        self.processVirtualMemoryInMB: Optional[float] = None
         if _havePsutil:
             self.percentNodeRamUsed = psutil.virtual_memory().percent
             self.processMemoryInMB = psutil.Process().memory_info().rss / (1024.0**2)
+            self.processVirtualMemoryInMB = psutil.Process().memory_info().vms / (
+                1024.0**2
+            )
 
     def __isub__(self, other):
         if self.percentNodeRamUsed is not None and other.percentNodeRamUsed is not None:
             self.percentNodeRamUsed -= other.percentNodeRamUsed
             self.processMemoryInMB -= other.processMemoryInMB
+            self.processVirtualMemoryInMB -= other.processVirtualMemoryInMB
         return self
 
 
diff --git a/armi/context.py b/armi/context.py
index 5f4205d0..71bb5b9b 100644
--- a/armi/context.py
+++ b/armi/context.py
@@ -15,10 +15,8 @@
 """
 Module containing global constants that reflect the executing context of ARMI.
 
-This contains information about the circumstatces under which an ARMI application is
-running. Things like the MPI environment, executing user, etc. live here. These are
-re-exported by the `armi` package, but live here so that import loops won't lead to
-as many issues.
+ARMI's global state information: operating system information, environment data, user data, memory
+parallelism, temporary storage locations, and if operational mode (interactive, gui, or batch).
 """
 from logging import DEBUG
 import datetime
@@ -29,14 +27,8 @@ import os
 import sys
 import time
 
-# h5py needs to be imported here, so that the disconnectAllHdfDBs() call that gets bound
-# to atexit below doesn't lead to a segfault on python exit. The Database3 module is
-# imported at call time, since it itself needs stuff that is initialized in this module
-# to import properly.  However, if that import leads to the first time that h5py is
-# imported in this process, doing so will cause a segfault. The theory here is that this
-# happens because the h5py extension module is not safe to import (for whatever reason)
-# when the python interpreter is in whatever state it's in when the atexit callbacks are
-# being invoked.  Importing early avoids this.
+# h5py needs to be imported here, so that the disconnectAllHdfDBs() call that gets bound to atexit
+# below doesn't lead to a segfault on python exit.
 #
 # Minimal code to reproduce the issue:
 #
@@ -53,25 +45,19 @@ import h5py  # noqa: F401
 BLUEPRINTS_IMPORTED = False
 BLUEPRINTS_IMPORT_CONTEXT = ""
 
-# App name is used when spawning new tasks that should invoke a specific ARMI
-# application. For instance, the framework provides some features to help with
-# submitting tasks to an HPC cluster. Sometimes these tasks are themselves only using
-# ARMI functionality, so running `python -m armi` is fine. Other times, the task is
-# specific to an application, requiring something like `python -m myArmiApp`
+# App name is used when spawning new tasks that should invoke a specific ARMI application. Sometimes
+# these tasks only use ARMI functionality, so running `python -m armi` is fine. Other times, the
+# task is specific to an application, requiring something like: `python -m myArmiApp`
 APP_NAME = "armi"
 
 
 class Mode(enum.Enum):
     """
-    Mode represents different run modes possible in ARMI.
+    Mode represents different run types possible in ARMI.
 
-    The modes can be Batch, Interactive, or GUI. In different modes, there are different
-    types of interactions possible.
-
-    Mode is generally auto-detected based on your terminal. It can also be set in
-    various CLI entry points, which are the implementations of
-    :py:class:`armi.cli.entryPoint.EntryPoint`. Lastly, each entry point has a
-    ``--batch`` command line argument that can force Batch mode.
+    The modes can be Batch, Interactive, or GUI. Mode is generally auto-detected based on your
+    terminal. It can also be set in various CLI entry points. Each entry point has a ``--batch``
+    command line argument that can force Batch mode.
     """
 
     BATCH = 1
@@ -93,10 +79,9 @@ DOC = os.path.abspath(os.path.join(PROJECT_ROOT, "doc"))
 USER = getpass.getuser()
 START_TIME = time.ctime()
 
-# Set batch mode if not a TTY, which means you're on a cluster writing to a stdout file
-# In this mode you cannot respond to prompts or anything
-# This does not work replabliy for both windows and linux so an os-specific solution is
-# applied
+# Set batch mode if not a TTY, which means you're on a cluster writing to a stdout file. In this
+# mode you cannot respond to prompts. (This does not work reliably for both Windows and Linux so an
+# os-specific solution is applied.)
 isatty = sys.stdout.isatty() if "win" in sys.platform else sys.stdin.isatty()
 CURRENT_MODE = Mode.INTERACTIVE if isatty else Mode.BATCH
 Mode.setMode(CURRENT_MODE)
@@ -104,8 +89,8 @@ Mode.setMode(CURRENT_MODE)
 MPI_COMM = None
 # MPI_RANK represents the index of the CPU that is running.
 # 0 is typically the primary CPU, while 1+ are typically workers.
-# MPI_SIZE is the total number of CPUs
 MPI_RANK = 0
+# MPI_SIZE is the total number of CPUs.
 MPI_SIZE = 1
 LOCAL = "local"
 MPI_NODENAME = LOCAL
@@ -113,13 +98,11 @@ MPI_NODENAMES = [LOCAL]
 
 
 try:
-    # Check for MPI
-    # The mpi4py module uses cPickle to serialize python objects in preparation for
-    # network transmission. Sometimes, when cPickle fails, it gives very cryptic error
-    # messages that do not help much. If you uncomment th following line, you can trick
-    # mpi4py into using the pure-python pickle module in place of cPickle and now you
-    # will generally get much more meaningful and useful error messages Then comment it
-    # back out because it's slow.
+    # Check for MPI. The mpi4py module uses cPickle to serialize python objects in preparation for
+    # network transmission. Sometimes, when cPickle fails, it gives very cryptic error messages that
+    # do not help much. If you uncomment th following line, you can trick mpi4py into using the
+    # pure-python pickle module in place of cPickle and now you will generally get much more
+    # meaningful and useful error messages Then comment it back out because it's slow.
     # import sys, pickle; sys.modules['cPickle'] = pickle
     from mpi4py import MPI
 
@@ -133,11 +116,11 @@ except ImportError:
     pass
 
 try:
-    # trying a windows approach
+    # trying a Windows approach
     APP_DATA = os.path.join(os.environ["APPDATA"], "armi")
     APP_DATA = APP_DATA.replace("/", "\\")
 except Exception:
-    # non-windows
+    # non-Windows
     APP_DATA = os.path.expanduser("~/.armi")
 
 if MPI_NODENAMES.index(MPI_NODENAME) == MPI_RANK:
@@ -150,13 +133,14 @@ if MPI_NODENAMES.index(MPI_NODENAME) == MPI_RANK:
         raise OSError("Directory doesn't exist {0}".format(APP_DATA))
 
 if MPI_COMM is not None:
-    MPI_COMM.barrier()  # make sure app data exists before workers proceed.
+    # Make sure app data exists before workers proceed.
+    MPI_COMM.barrier()
 
 MPI_DISTRIBUTABLE = MPI_SIZE > 1
 
 _FAST_PATH = os.path.join(os.getcwd())
 """
-A directory available for high-performance I/O
+A directory available for high-performance I/O.
 
 .. warning:: This is not a constant and can change at runtime.
 """
@@ -169,17 +153,16 @@ def activateLocalFastPath() -> None:
     """
     Specify a local temp directory to be the fast path.
 
-    ``FAST_PATH`` is often a local hard drive on a cluster node. It's a high-performance
+    ``FAST_PATH`` is often a local hard drive on a cluster node. It should be a high-performance
     scratch space. Different processors on the same node should have different fast paths.
-    Some old code may MPI_RANK-dependent folders/filenames as well, but this is no longer
-    necessary.
 
-    .. warning:: This path will be obliterated when the job ends so be careful.
+    Notes
+    -----
+    This path will be obliterated when the job ends.
 
-    Note also
-    that this path is set at import time, so if a series of unit tests come through that
-    instantiate one operator after the other, the path will already exist the second time.
-    The directory is created in the Operator constructor.
+    This path is set at import time, so if a series of unit tests come through that instantiate one
+    operator after the other, the path will already exist the second time. The directory is created
+    in the Operator constructor.
     """
     global _FAST_PATH, _FAST_PATH_IS_TEMPORARY, APP_DATA
 
@@ -205,9 +188,8 @@ def getFastPath() -> str:
 
     Notes
     -----
-    It's too dangerous to use ``FAST_PATH`` directly as it can change between import and runtime.
-    For example, a module that does ``from armi.context import FAST_PATH`` is disconnected from the
-    official ``FAST_PATH`` controlled by this module.
+    This exists because it's dangerous to use ``FAST_PATH`` directly. as it can change between
+    import and runtime.
     """
     return _FAST_PATH
 
@@ -216,12 +198,12 @@ def cleanTempDirs(olderThanDays=None):
     """
     Clean up temporary files after a run.
 
-    The Windows HPC system sends a SIGBREAK signal when the user cancels a job, which is NOT handled
-    by ``atexit``. Notably SIGBREAK doesn't exist off Windows. For the SIGBREAK signal to work with
-    a Microsoft HPC, the ``TaskCancelGracePeriod`` option must be configured to be non-zero. This
-    sets the period between SIGBREAK and SIGTERM/SIGINT. To do cleanups in this case, we must use
-    the ``signal`` module. Actually, even then it does not work because MS ``mpiexec`` does not pass
-    signals through.
+    Some Windows HPC systems send a SIGBREAK signal when the user cancels a job, which is NOT
+    handled by ``atexit``. Notably, SIGBREAK does not exist outside Windows. For the SIGBREAK signal
+    to work with a Windows HPC, the ``TaskCancelGracePeriod`` option must be configured to be non-
+    zero. This sets the period between SIGBREAK and SIGTERM/SIGINT. To do cleanups in this case, we
+    must use the ``signal`` module. Actually, even then it does not work because MS ``mpiexec`` does
+    not pass signals through.
 
     Parameters
     ----------
diff --git a/armi/reactor/converters/blockConverters.py b/armi/reactor/converters/blockConverters.py
index 62f7873f..a74491d8 100644
--- a/armi/reactor/converters/blockConverters.py
+++ b/armi/reactor/converters/blockConverters.py
@@ -311,7 +311,8 @@ class MultipleComponentMerger(BlockConverter):
                 soluteName, self.solventName, minID=self.specifiedMinID
             )
         solvent = self._sourceBlock.getComponentByName(self.solventName)
-        BlockConverter._verifyExpansion(self, self.soluteNames, solvent)
+        if solvent.__class__ is not components.DerivedShape:
+            BlockConverter._verifyExpansion(self, self.soluteNames, solvent)
         return self._sourceBlock
 
 


Test patch:
diff --git a/armi/bookkeeping/db/tests/test_databaseInterface.py b/armi/bookkeeping/db/tests/test_databaseInterface.py
index 50cb7b4d..7b106b27 100644
--- a/armi/bookkeeping/db/tests/test_databaseInterface.py
+++ b/armi/bookkeeping/db/tests/test_databaseInterface.py
@@ -350,7 +350,8 @@ class TestDatabaseReading(unittest.TestCase):
         # than the original input file. This allows settings to be
         # changed in memory like this and survive for testing.
         newSettings = {"verbosity": "extra"}
-        newSettings["nCycles"] = 2
+        cls.nCycles = 2
+        newSettings["nCycles"] = cls.nCycles
         newSettings["burnSteps"] = 2
         o, r = loadTestReactor(customSettings=newSettings)
         reduceTestReactorRings(r, o.cs, 3)
@@ -510,6 +511,18 @@ class TestDatabaseReading(unittest.TestCase):
 
         assert_allclose(numDensVec1, numDensVec2)
 
+    def test_timesteps(self):
+        with Database3(self.dbName, "r") as db:
+            # build time steps in the DB file
+            timesteps = []
+            for cycle in range(self.nCycles):
+                for bStep in range(3):
+                    timesteps.append(f"/c0{cycle}n0{bStep}")
+            timesteps.append("/c01n02EOL")
+
+            # verify the timesteps are correct, including the EOL
+            self.assertEqual(list(db.keys()), timesteps)
+
 
 class TestBadName(unittest.TestCase):
     def test_badDBName(self):
diff --git a/armi/operators/tests/test_operators.py b/armi/operators/tests/test_operators.py
index 6a0638bd..e828989f 100644
--- a/armi/operators/tests/test_operators.py
+++ b/armi/operators/tests/test_operators.py
@@ -207,15 +207,28 @@ class OperatorTests(unittest.TestCase):
         self.assertIn("history", interfaceNames)
         self.assertNotIn("xsGroups", interfaceNames)
 
-        # Test EOL
-        interfaces = self.o.getActiveInterfaces("EOL")
-        self.assertEqual(interfaces[-1].name, "main")
-
         # Test Coupled
         interfaces = self.o.getActiveInterfaces("Coupled")
         for test, ref in zip(interfaces, self.activeInterfaces):
             self.assertEqual(test.name, ref.name)
 
+        # Test EOL
+        interfaces = self.o.getActiveInterfaces("EOL")
+        self.assertEqual(interfaces[-1].name, "main")
+
+        # Test excludedInterfaceNames
+        excludedInterfaceNames = ["fissionProducts", "fuelHandler", "xsGroups"]
+        interfaces = self.o.getActiveInterfaces(
+            "EOL", excludedInterfaceNames=excludedInterfaceNames
+        )
+        interfaceNames = [ii.name for ii in interfaces]
+        self.assertIn("history", interfaceNames)
+        self.assertIn("main", interfaceNames)
+        self.assertIn("snapshot", interfaceNames)
+        self.assertNotIn("fissionProducts", interfaceNames)
+        self.assertNotIn("fuelHandler", interfaceNames)
+        self.assertNotIn("xsGroups", interfaceNames)
+
     def test_loadStateError(self):
         """The ``loadTestReactor()`` test tool does not have any history in the DB to load from."""
         # a first, simple test that this method fails correctly
diff --git a/armi/reactor/converters/tests/test_blockConverter.py b/armi/reactor/converters/tests/test_blockConverter.py
index 9218a73f..95402aa9 100644
--- a/armi/reactor/converters/tests/test_blockConverter.py
+++ b/armi/reactor/converters/tests/test_blockConverter.py
@@ -81,6 +81,22 @@ class TestBlockConverter(unittest.TestCase):
         self.assertNotIn(soluteName, convertedBlock.getComponentNames())
         self._checkAreaAndComposition(block, convertedBlock)
 
+    def test_dissolveMultiple(self):
+        """Test dissolving multiple components into another."""
+        self._test_dissolve_multi(loadTestBlock(), ["wire", "clad"], "coolant")
+        self._test_dissolve_multi(
+            loadTestBlock(), ["inner liner", "outer liner"], "clad"
+        )
+
+    def _test_dissolve_multi(self, block, soluteNames, solventName):
+        converter = blockConverters.MultipleComponentMerger(
+            block, soluteNames, solventName
+        )
+        convertedBlock = converter.convert()
+        for soluteName in soluteNames:
+            self.assertNotIn(soluteName, convertedBlock.getComponentNames())
+        self._checkAreaAndComposition(block, convertedBlock)
+
     def test_build_NthRing(self):
         """Test building of one ring."""
         RING = 6
diff --git a/armi/tests/test_context.py b/armi/tests/test_context.py
new file mode 100644
index 00000000..69e737fd
--- /dev/null
+++ b/armi/tests/test_context.py
@@ -0,0 +1,37 @@
+# Copyright 2024 TerraPower, LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Serial tests for the Context module."""
+import unittest
+
+from armi import context
+
+
+class TestContextSerial(unittest.TestCase):
+    """Serial tests for the Context module."""
+
+    @unittest.skipIf(context.MPI_SIZE > 1, "Serial test only")
+    def test_rank(self):
+        self.assertEqual(context.MPI_RANK, 0)
+        self.assertEqual(context.MPI_SIZE, 1)
+
+    @unittest.skipIf(context.MPI_SIZE > 1, "Serial test only")
+    def test_nonNoneData(self):
+        self.assertGreater(len(context.APP_DATA), 0)
+        self.assertGreater(len(context.DOC), 0)
+        self.assertGreater(len(context.getFastPath()), 0)
+        self.assertGreater(len(context.PROJECT_ROOT), 0)
+        self.assertGreater(len(context.RES), 0)
+        self.assertGreater(len(context.ROOT), 0)
+        self.assertGreater(len(context.USER), 0)
diff --git a/armi/tests/test_mpiFeatures.py b/armi/tests/test_mpiFeatures.py
index 60f9d9d1..3682d568 100644
--- a/armi/tests/test_mpiFeatures.py
+++ b/armi/tests/test_mpiFeatures.py
@@ -351,3 +351,21 @@ class MpiPathToolsTests(unittest.TestCase):
             pathTools.cleanPath(dir3, mpiRank=context.MPI_RANK)
             MPI_COMM.barrier()
             self.assertFalse(os.path.exists(dir3))
+
+
+class TestContextMpi(unittest.TestCase):
+    """Parallel tests for the Context module."""
+
+    @unittest.skipIf(context.MPI_SIZE <= 1 or MPI_EXE is None, "Parallel test only")
+    def test_rank(self):
+        self.assertGreater(context.MPI_RANK, -1)
+
+    @unittest.skipIf(context.MPI_SIZE <= 1 or MPI_EXE is None, "Parallel test only")
+    def test_nonNoneData(self):
+        self.assertGreater(len(context.APP_DATA), 0)
+        self.assertGreater(len(context.DOC), 0)
+        self.assertGreater(len(context.getFastPath()), 0)
+        self.assertGreater(len(context.PROJECT_ROOT), 0)
+        self.assertGreater(len(context.RES), 0)
+        self.assertGreater(len(context.ROOT), 0)
+        self.assertGreater(len(context.USER), 0)