Repository: terrapower/armi
Pull request #1258 (2023-05-04 23:10:25)

Fix a bug in database compare.
## Description
---

There's a line in compareDB3.py that tries to call a method that doesn't exist, `DiffResults.add()`

This has been around for a long time, but we have somehow just coincidentally not been hitting it.

Also, another call to `addDiff` needed to be updated to pass `numpy.inf` as a scalar instead of a list.

## Checklist

<!--
    You (the pull requester) should put an `x` in the boxes below you have completed.
    If you're unsure about any of them, don't hesitate to ask. We're here to help!
    Learn what a "good PR" looks like here: https://terrapower.github.io/armi/developer/tooling.html#good-pull-requests
-->

- [x] This PR has only one purpose or idea.
- [x] Tests have been added/updated to verify that the new/changed code works.

<!-- 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/release/index.html) (location `doc/release/0.X.rst`) are up-to-date with any bug fixes or new features.
- [x] The documentation is still up-to-date in the `doc` folder.
- [x] The dependencies are still up-to-date in `setup.py`.



mgjarrett: I was able to add a unit test for one of the bad function calls.

For the second one, where the name of the function was wrong (`add()` instead of `addDiff()`), I have not been able to get the unit test to hit the line. To get here, we have to read some data `src` from an HDF5 dataset, iterate throught `src.tolist()` which ostensibly creates regular python `list`s, and then check if any of the items created by `tolist()` are `numpy.ndarray` instead of a regular `list`. I don't know how to make this happen, and I'm not sure how I happened upon it yesterday. For now I'll settle for leaving the line uncovered; I know the fix works because I tried it on my in-use case that was failing.

Patch:
diff --git a/armi/bookkeeping/db/compareDB3.py b/armi/bookkeeping/db/compareDB3.py
index 47395db5..12fd02e5 100644
--- a/armi/bookkeeping/db/compareDB3.py
+++ b/armi/bookkeeping/db/compareDB3.py
@@ -323,13 +323,13 @@ def _diffSpecialData(
     compName = refData.name.split("/")[-2]
 
     nDiffs = _compareSets(
-        set(srcData.attrs.keys()), set(refData.attrs.keys()), "formatting data"
+        set(srcData.attrs.keys()), set(refData.attrs.keys()), out, "formatting data"
     )
     keysMatch = nDiffs == 0
     diffResults.addStructureDiffs(nDiffs)
 
     if not keysMatch:
-        diffResults.addDiff(name, name, [numpy.inf], [numpy.inf], [numpy.inf])
+        diffResults.addDiff(name, name, numpy.inf, numpy.inf, numpy.inf)
         return
 
     if srcData.attrs.get("dict", False):
@@ -377,7 +377,9 @@ def _diffSpecialData(
         if isinstance(dSrc, numpy.ndarray) and isinstance(dRef, numpy.ndarray):
             if dSrc.shape != dRef.shape:
                 out.writeln("Shapes did not match for {}".format(refData))
-                diffResults.add([numpy.inf], [numpy.inf], [numpy.inf], [numpy.inf])
+                diffResults.addDiff(
+                    compName, paramName, numpy.inf, numpy.inf, numpy.inf
+                )
                 return
 
             # make sure not to try to compare empty arrays. Numpy is mediocre at
diff --git a/armi/physics/neutronics/fissionProductModel/fissionProductModel.py b/armi/physics/neutronics/fissionProductModel/fissionProductModel.py
index 3e37a997..c321f2b4 100644
--- a/armi/physics/neutronics/fissionProductModel/fissionProductModel.py
+++ b/armi/physics/neutronics/fissionProductModel/fissionProductModel.py
@@ -247,7 +247,8 @@ class FissionProductModel(interfaces.Interface):
             else:
                 independentLFPs = self.getGlobalLumpedFissionProducts().duplicate()
                 b.setLumpedFissionProducts(independentLFPs)
-            self._initializeIsoDensities(b)
+            if self.allBlocksNeedAllNucs:
+                self._initializeIsoDensities(b)
 
     def getGlobalLumpedFissionProducts(self):
         r"""
diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst
index 5cdd58a5..47416f0b 100644
--- a/doc/release/0.2.rst
+++ b/doc/release/0.2.rst
@@ -24,6 +24,7 @@ What's new in ARMI
 #. New cross section group representative block type for 1D cylindrical models. (`PR#1238 <https://github.com/terrapower/armi/pull/1238>`_)
 #. Store the axial expansion target component name as a block parameter. (`PR#1256 https://github.com/terrapower/armi/pull/1256`_) 
 #. When using non-uniform mesh, detailed fission/activation products have cross sections generated to avoid blocks without xs data. (`PR#1257 <https://github.com/terrapower/armi/pull/1257>`_)
+#. Fix a bug in database comparison. (`PR#1258 <https://github.com/terrapower/armi/pull/1258>`_)
 #. TBD
 
 Bug fixes


Test patch:
diff --git a/armi/bookkeeping/db/tests/test_comparedb3.py b/armi/bookkeeping/db/tests/test_comparedb3.py
index 0738d6df..4b404163 100644
--- a/armi/bookkeeping/db/tests/test_comparedb3.py
+++ b/armi/bookkeeping/db/tests/test_comparedb3.py
@@ -235,6 +235,17 @@ class TestCompareDB3(unittest.TestCase):
                 _diffSpecialData(refData4, srcData5, out, dr)
                 self.assertIn("Unable to unpack special data for paramName", e)
 
+            # make an H5 datasets that will add a np.inf diff because keys don't match
+            f6 = h5py.File("test_diffSpecialData6.hdf5", "w")
+            refData6 = f6.create_dataset("numberDensities", data=a2)
+            refData6.attrs["shapes"] = "2"
+            refData6.attrs["numDens"] = a2
+            f7 = h5py.File("test_diffSpecialData7.hdf5", "w")
+            srcData7 = f7.create_dataset("densities", data=a2)
+            srcData7.attrs["colors"] = "2"
+            srcData7.attrs["numberDens"] = a2
+            _diffSpecialData(refData6, srcData7, out, dr)
+
     def test_diffSimpleData(self):
         dr = DiffResults(0.01)
 
diff --git a/armi/physics/neutronics/fissionProductModel/tests/test_fissionProductModel.py b/armi/physics/neutronics/fissionProductModel/tests/test_fissionProductModel.py
index 36439fc6..98a03a47 100644
--- a/armi/physics/neutronics/fissionProductModel/tests/test_fissionProductModel.py
+++ b/armi/physics/neutronics/fissionProductModel/tests/test_fissionProductModel.py
@@ -82,6 +82,17 @@ class TestFissionProductModelLumpedFissionProducts(unittest.TestCase):
             else:
                 self.assertTrue(b._lumpedFissionProducts is None)
 
+        # now check if all depletable blocks do not have all nuclides if not detailedAxialExpansion
+        fpModel.allBlocksNeedAllNucs = False
+        fpModel.interactBOL()
+        allNucsInProblem = set(r.blueprints.allNuclidesInProblem)
+        for b in r.core.getBlocks():
+            if isDepletable(b):
+                if len(allNucsInProblem - set(b.getNuclides())) > 0:
+                    break
+        else:
+            self.assertTrue(False, "All blocks have all nuclides!")
+
         # now check if detailed axial expansion that all blocks have ALL nuclides
         fpModel.allBlocksNeedAllNucs = True
         fpModel.interactBOL()