Repository: terrapower/armi
Pull request #2051 (2025-01-11 06:57:57)

Update the check for a jagged array during `_writeParams`
## What is the change?

Scenario where `jagged` evaluates to `False` has a loophole (with py313 and numpy 2.2.1): single-element numpy arrays mixed into a list with non-array entries fails in `_writeParams`. (Edit: it wasn't a loophole...it's that I was previously using a very old version of numpy that allowed jagged arrays, and so the parameter that ended up as a jagged array slipped through the cracks.)

This updates the check for `jagged` to account for this scenario by editing `_getShape`.

## Why is the change being made?

I was getting this failure sometimes while running cases (with py313 and numpy 2.2.1):

```
    JaggedArray(temp, paramDef.name) if jagged else np.array(temp)
                                                    ~~~~~~~~^^^^^^
ValueError: setting an array element with a sequence. The requested array has an inhomogeneous shape after 1 dimensions. The detected shape was (782,) + inhomogeneous part.
```

This is because the logic for determining whether something is a jagged array misses the scenario where single element numpy arrays are mixed with non array/list entries. The code in `_getShape` evaluates to `(1,)` for all these array entries AS WELL AS for `int`, `float`, or `None`, which means the `jagged` check evaluates to `False`. Then `np.array(temp)` fails with the above error.  So I have the `_getShape` helper function return `1` for the latter scenario. Then the set for a list of elements mixed with single element arrays is `{(1,), 1}`, and causes `jagged` to be `True`.

### Jagged or not jagged, that is the question

We can nix the above update and keep `jagged` as `False` by editing the temp list like so:

``` 
temp = [
    t.item() if isinstance(t, np.ndarray) and len(t) == 1 else t
    for t in temp
]
``` 

I'm just assuming we want `jagged` to be true, so went with the solution in the PR. Also, this edits the list after the fact. I don't care either way.

---

## 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 -->

- [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`.

john-science: Hmmm... it makes sense.

1. Can you add a unit test to show me/everybody what the new expected behavior is in this edge case?
2. Do we need to run downstream CI on this PR?
john-science: @opotowsky 

I think, perhaps, the method you touch is unused.  It is here:

https://github.com/terrapower/armi/blob/85053e36541b1f32256ecf70cab2adb2982d091a/armi/bookkeeping/db/database.py#L917

But the method is called `_writeParams()`, but it is only used in one place in the method `Database.writeToDB()`:

https://github.com/terrapower/armi/blob/85053e36541b1f32256ecf70cab2adb2982d091a/armi/bookkeeping/db/database.py#L671-L681

But, if you look, the method `_writeParams()` doesn't _actually_ write parameters any more (perhaps it did once).  All it does is return a value that is not used.

Do you agree?
opotowsky: @john-science it doesn't return anything cause all it does is write things to the DB. Towards the bottom of the method you see this:
<img width="631" alt="image" src="https://github.com/user-attachments/assets/9b564f46-3d32-4e2f-9007-9a62ca812c5b" />

john-science: Okay, one option is to move this private function out to being a regular, private method: `Database._getArrayShape`:

https://github.com/terrapower/armi/blob/85053e36541b1f32256ecf70cab2adb2982d091a/armi/bookkeeping/db/database.py#L918-L925

And then add some unit tests.

Patch:
diff --git a/armi/bookkeeping/db/database.py b/armi/bookkeeping/db/database.py
index 672e905f..1a19dbcd 100644
--- a/armi/bookkeeping/db/database.py
+++ b/armi/bookkeeping/db/database.py
@@ -914,16 +914,18 @@ class Database:
 
         return comp
 
-    def _writeParams(self, h5group, comps) -> tuple:
-        def _getShape(arr: [np.ndarray, List, Tuple]):
-            """Get the shape of a np.ndarray, list, or tuple."""
-            if isinstance(arr, np.ndarray):
-                return arr.shape
-            elif isinstance(arr, (list, tuple)):
-                return (len(arr),)
-            else:
-                return (1,)
+    @staticmethod
+    def _getArrayShape(arr: [np.ndarray, List, Tuple]):
+        """Get the shape of a np.ndarray, list, or tuple."""
+        if isinstance(arr, np.ndarray):
+            return arr.shape
+        elif isinstance(arr, (list, tuple)):
+            return (len(arr),)
+        else:
+            # not a list, tuple, or array (likely int, float, or None)
+            return 1
 
+    def _writeParams(self, h5group, comps) -> tuple:
         c = comps[0]
         groupName = c.__class__.__name__
         if groupName not in h5group:
@@ -969,7 +971,7 @@ class Database:
                 else:
                     # check if temp is a jagged array
                     if any(isinstance(x, (np.ndarray, list)) for x in temp):
-                        jagged = len(set([_getShape(x) for x in temp])) != 1
+                        jagged = len(set([self._getArrayShape(x) for x in temp])) != 1
                     else:
                         jagged = False
                     data = (
diff --git a/doc/release/0.5.rst b/doc/release/0.5.rst
index e005d82f..d487bb64 100644
--- a/doc/release/0.5.rst
+++ b/doc/release/0.5.rst
@@ -19,6 +19,7 @@ API Changes
 
 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>`_)
 #. Fixing scaling of volume-integrated parameters on edge assembleis. (`PR#2060 <https://github.com/terrapower/armi/pull/2060>`_)
 #. TBD


Test patch:
diff --git a/armi/bookkeeping/db/tests/test_database3.py b/armi/bookkeeping/db/tests/test_database3.py
index 91246ac5..09491fb9 100644
--- a/armi/bookkeeping/db/tests/test_database3.py
+++ b/armi/bookkeeping/db/tests/test_database3.py
@@ -23,6 +23,7 @@ import h5py
 import numpy as np
 
 from armi.bookkeeping.db import _getH5File, database, loadOperator
+from armi.bookkeeping.db.database import Database
 from armi.bookkeeping.db.databaseInterface import DatabaseInterface
 from armi.bookkeeping.db.jaggedArray import JaggedArray
 from armi.reactor import parameters
@@ -283,6 +284,19 @@ class TestDatabaseSmaller(unittest.TestCase):
         roundTrip = database.unpackSpecialData(packed, attrs, "testing")
         self._compareArrays(data, roundTrip)
 
+    def test_getArrayShape(self):
+        """Tests a helper method for ``_writeParams``."""
+        base = [1, 2, 3, 4]
+        self.assertEqual(Database._getArrayShape(base), (4,))
+        self.assertEqual(Database._getArrayShape(tuple(base)), (4,))
+        arr = np.array(base)
+        self.assertEqual(Database._getArrayShape(arr), (4,))
+        arr = np.array([base])
+        self.assertEqual(Database._getArrayShape(arr), (1, 4))
+        # not array type
+        self.assertEqual(Database._getArrayShape(1), 1)
+        self.assertEqual(Database._getArrayShape(None), 1)
+
     def test_writeToDB(self):
         """Test writing to the database.