Repository: terrapower/armi
Pull request #1777 (2024-07-12 16:41:40)

Fixing two semi-broken unit tests
## What is the change?

I added two `db.close()` statements to unit tests in `armi/cli/tests/test_runEntryPoint.py`.

## Why is the change being made?

While these tests were nominally passing, we have a particularly sensitive cluster where these tests were failing, because of the interaction between unclosed database files and the temporary directories that were housing them.

This was just a mistake; not closing DBs in unit tests.

---

## 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`.


Patch:
diff --git a/armi/bookkeeping/db/jaggedArray.py b/armi/bookkeeping/db/jaggedArray.py
index 3135f7da..c28b9874 100644
--- a/armi/bookkeeping/db/jaggedArray.py
+++ b/armi/bookkeeping/db/jaggedArray.py
@@ -173,17 +173,19 @@ class JaggedArray:
             List of numpy arrays with varying dimensions (i.e., jagged arrays)
         """
         unpackedJaggedData: List[Optional[np.ndarray]] = []
-        numElements = len(self.offsets) + len(self.nones)
+        shapeIndices = [i for i, x in enumerate(self.shapes) if sum(x) != 0]
+        numElements = len(shapeIndices) + len(self.nones)
         j = 0  # non-None element counter
         for i in range(numElements):
             if i in self.nones:
                 unpackedJaggedData.append(None)
             else:
+                k = shapeIndices[j]
                 unpackedJaggedData.append(
                     np.ndarray(
-                        self.shapes[j],
+                        self.shapes[k],
                         dtype=self.dtype,
-                        buffer=self.flattenedArray[self.offsets[j] :],
+                        buffer=self.flattenedArray[self.offsets[k] :],
                     )
                 )
                 j += 1
diff --git a/armi/context.py b/armi/context.py
index 4927b025..0a36b00e 100644
--- a/armi/context.py
+++ b/armi/context.py
@@ -206,9 +206,9 @@ 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.
+    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.
     """
     return _FAST_PATH
 
@@ -217,19 +217,17 @@ 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.
+    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.
 
     Parameters
     ----------
     olderThanDays: int, optional
-        If provided, deletes other ARMI directories if they are older than the requested
-        time.
+        If provided, deletes other ARMI directories if they are older than the requested time.
     """
     from armi import runLog
     from armi.utils.pathTools import cleanPath
@@ -259,12 +257,14 @@ def cleanTempDirs(olderThanDays=None):
 
 def cleanAllArmiTempDirs(olderThanDays: int) -> None:
     """
-    Delete all ARMI-related files from other unrelated runs after `olderThanDays` days (in
-    case this failed on earlier runs).
-
-    .. warning:: This will break any concurrent runs that are still running.
+    Delete all ARMI-related files from other unrelated runs after `olderThanDays` days (in case this
+    failed on earlier runs).
 
     This is a useful utility in HPC environments when some runs crash sometimes.
+
+    Warning
+    -------
+    This will break any concurrent runs that are still running.
     """
     from armi.utils.pathTools import cleanPath
 
@@ -294,13 +294,12 @@ def disconnectAllHdfDBs() -> None:
 
     Notes
     -----
-    This is a hack to help ARMI exit gracefully when the garbage collector and h5py have
-    issues destroying objects. After lots of investigation, the root cause for why this
-    was having issues was never identified. It appears that when several HDF5 files are
-    open in the same run (e.g.  when calling armi.init() multiple times from a
-    post-processing script), when these h5py File objects were closed, the garbage
-    collector would raise an exception related to the repr'ing the object. We
-    get around this by using the garbage collector to manually disconnect all open HdfDB
+    This is a hack to help ARMI exit gracefully when the garbage collector and h5py have issues
+    destroying objects. After lots of investigation, the root cause for why this was having issues
+    was never identified. It appears that when several HDF5 files are open in the same run (e.g.
+    when calling armi.init() multiple times from a post-processing script), when these h5py File
+    objects were closed, the garbage collector would raise an exception related to the repr'ing the
+    object. We get around this by using the garbage collector to manually disconnect all open HdfDB
     objects.
     """
     from armi.bookkeeping.db import Database3
diff --git a/armi/utils/pathTools.py b/armi/utils/pathTools.py
index d0594736..0f6104f7 100644
--- a/armi/utils/pathTools.py
+++ b/armi/utils/pathTools.py
@@ -25,6 +25,16 @@ import shutil
 from armi import context
 from armi import runLog
 
+DO_NOT_CLEAN_PATHS = [
+    "armiruns",
+    "failedruns",
+    "mc2run",
+    "mongoose",
+    "shufflebranches",
+    "snapshot",
+    "tests",
+]
+
 
 def armiAbsPath(*pathParts):
     """
@@ -194,12 +204,12 @@ def moduleAndAttributeExist(pathAttr):
 def cleanPath(path, mpiRank=0):
     """Recursively delete a path.
 
-    !!! careful with this !!! It can delete the entire cluster.
+    !!! Be careful with this !!! It can delete the entire cluster.
 
-    We add copious os.path.exists checks in case an MPI set of things is trying to delete everything at the same time.
-    Always check filenames for some special flag when calling this, especially
-    with full permissions on the cluster. You could accidentally delete everyone's work
-    with one misplaced line! This doesn't ask questions.
+    We add copious os.path.exists checks in case an MPI set of things is trying to delete everything
+    at the same time. Always check filenames for some special flag when calling this, especially
+    with full permissions on the cluster. You could accidentally delete everyone's work with one
+    misplaced line! This doesn't ask questions.
 
     Safety nets include an allow-list of paths.
 
@@ -214,15 +224,7 @@ def cleanPath(path, mpiRank=0):
     if not os.path.exists(path):
         return True
 
-    for validPath in [
-        "armiruns",
-        "failedruns",
-        "mc2run",
-        "mongoose",
-        "shufflebranches",
-        "snapshot",
-        "tests",
-    ]:
+    for validPath in DO_NOT_CLEAN_PATHS:
         if validPath in path.lower():
             valid = True
 


Test patch:
diff --git a/armi/bookkeeping/db/tests/test_jaggedArray.py b/armi/bookkeeping/db/tests/test_jaggedArray.py
index 89d6c53f..d519bc7f 100644
--- a/armi/bookkeeping/db/tests/test_jaggedArray.py
+++ b/armi/bookkeeping/db/tests/test_jaggedArray.py
@@ -47,6 +47,48 @@ class TestJaggedArray(unittest.TestCase):
         flatArray = JaggedArray.flatten(testdata)
         self.assertEqual(flatArray, [1, 2, 3, 4, 5, None, 6, 7, 8, 9])
 
+    def test_backwardsCompatible(self):
+        """
+        Test that the new JaggedArray can unpack the old database jagged data format.
+
+        The "old" database format contains shapes and offsets for locations that have None.
+        The "new" database format only contains shapes and offsets for non-None values.
+        The "new" unpacking routine is able to read either format.
+        """
+        paramName = "test_old"
+        data = [[1, 2], None, [3, 4, 5], None, None, [6, 7, 8, 9]]
+        flattenedArray = numpy.array([1, 2, 3, 4, 5, 6, 7, 8, 9])
+        shapes = [(2,), (0,), (3,), (0,), (0,), (4,)]
+        offsets = [0, 2, 2, 5, 5, 5, 5]
+        nones = [1, 3, 4]
+        h5file = "test_oldFormat.h5"
+        with h5py.File(h5file, "w") as hf:
+            dset = hf.create_dataset(
+                data=flattenedArray,
+                name=paramName,
+            )
+            dset.attrs["jagged"] = True
+            dset.attrs["offsets"] = offsets
+            dset.attrs["shapes"] = shapes
+            dset.attrs["noneLocations"] = nones
+
+        with h5py.File(h5file, "r") as hf:
+            dataset = hf[paramName]
+            values = dataset[()]
+            offsets = dataset.attrs["offsets"]
+            shapes = dataset.attrs["shapes"]
+            nones = dataset.attrs["noneLocations"]
+
+        roundTrip = JaggedArray.fromH5(
+            values,
+            offsets,
+            shapes,
+            nones,
+            dtype=flattenedArray.dtype,
+            paramName=paramName,
+        )
+        self._compareArrays(data, roundTrip)
+
     def _compareRoundTrip(self, data, paramName):
         """Make sure that data is unchanged by packing/unpacking."""
         jaggedArray = JaggedArray(data, paramName)
diff --git a/armi/cli/tests/test_runEntryPoint.py b/armi/cli/tests/test_runEntryPoint.py
index fa6136bb..93cc995d 100644
--- a/armi/cli/tests/test_runEntryPoint.py
+++ b/armi/cli/tests/test_runEntryPoint.py
@@ -252,6 +252,8 @@ class TestExtractInputs(unittest.TestCase):
                 self.assertIn("Writing settings to", mock.getStdout())
                 self.assertIn("Writing blueprints to", mock.getStdout())
 
+            db.close()
+
 
 class TestInjectInputs(unittest.TestCase):
     def test_injectInputsBasics(self):
@@ -453,3 +455,5 @@ class TestVisFileEntryPointCommand(unittest.TestCase):
             self.assertIsNone(vf.args.min_node)
             self.assertIsNone(vf.args.max_node)
             self.assertEqual(vf.args.output_name, "test_visFileEntryPointBasics")
+
+            self.db.close()