Repository: terrapower/armi

Issue:
HTGR pin lattice not well mapped during vis-file
I'm attempting to model an HTGR minicore and having a few issues / sticking points. The core is a simple 2-ring hex grid with a prismatic-like pin lattice
**Core**
![core-map](https://user-images.githubusercontent.com/19477741/108424956-ae93a980-71ee-11eb-8a9a-a48473b1a106.png)
**Pin lattice**
![pin-lattice](https://user-images.githubusercontent.com/19477741/108424957-ae93a980-71ee-11eb-890d-4c04470da455.png)

<details>
<summary>Blueprints.yaml</summary>

```yaml
blocks:
    fuel: &block_fuel
        grid name: pins
        duct: &comp_duct
            shape: Hexagon
            material: HT9
            Tinput: 600
            Thot: 600
            ip: 32
            op: 32.2
        fuel:
            shape: Circle
            material: UZr
            Tinput: 600
            Thot: 600
            id: 0.0
            od: 0.8
            latticeIDs: [FP]
        clad:
            shape: Circle
            material: Zr
            Tinput: 600
            Thot: 600
            id: fuel.od
            od: 0.9
            latticeIDs: [FP]
        pitch:
            shape: Hexagon
            material: Void
            Tinput: 600
            Thot: 600
            ip: 3
            op: 3
            latticeIDs: [FP, CL]
        cool pin:
            shape: Circle
            material: Graphite
            Tinput: 600
            Thot: 600
            id: 0
            od: fuel.od
            latticeIDs: [CL]
        coolant:
            shape: DerivedShape
            material: Graphite
            Tinput: 600
            Thot: 600
assemblies:
    heights: &heights
        - 50.0
    axial mesh points: &mesh
        - 2
    fuel:
        specifier : F
        blocks : &fuel_blocks
            - *block_fuel
        height: *heights
        axial mesh points: *mesh
        material modifications:
            U235_wt_frac:
                - 0.127
        xs types : &IC_xs
            - A
systems:
    core:
        grid name: core
        origin:
            x: 0.0
            y: 0.0
            z: 0.0
grids:
    !include minicore-coremap.yaml
```
</details>

<details>
<summary>Coremap.yaml</summary>

```yaml
core:
  geom: hex
  symmetry: full
  lattice map: |
    F
      F  F
    F
    F  F
      F
pins:
  geom: hex
  symmetry: full
  lattice map: |
    -   -   FP
      -   FP  FP
    -   CL  CL  CL
      FP  FP  FP  FP
    FP  FP  FP  FP  FP
      CL  CL  CL  CL
    FP  FP  FP  FP  FP
      FP  FP  FP  FP
    CL  CL  CL  CL  CL
      FP  FP  FP  FP
    FP  FP  FP  FP  FP
      CL  CL  CL  CL
    FP  FP  FP  FP  FP
      FP  FP  FP  FP
        CL  CL  CL
          FP  FP
            FP
```
</details>
ARMI 5b7d215 doesn't have any issue running the model and writing a datafile. The output of armi vis-file minicore.h5 yields

```
                       ---------------------------------------------------
                      |             _      ____     __  __    ___         |
                      |            / \    |  _ \   |  \/  |  |_ _|        |
                      |           / _ \   | |_) |  | |\/| |   | |         |
                      |          / ___ \  |  _ <   | |  | |   | |         |
                      |         /_/   \_\ |_| \_\  |_|  |_|  |___|        |
                      |         Advanced  Reactor  Modeling Interface     |
                       ---------------------------------------------------
                                      Version 0.1.6

[info] Creating visualization file for cycle 0, time node 0...
[info] Loading reactor state for time node (0, 0)
[info] Applying <Migration from 0.1.2: Removes now-invalid `centers:` lines fro...
[info] Removing `centers:` sections.
[info] Applying <Migration from 0.1.2: Update elemental nuclide flags....
=========== Initializing Mesh, Assembly Zones, and Nuclide Categories ===========
[info] Nuclide categorization for cross section temperature assignments:
       ------------------  -----------------------------------------------------------
       Nuclide Category    Nuclides
       ------------------  -----------------------------------------------------------
       Fuel                CM245, LFP41, PU236, PU241, DUMP1, ZR90, U238, ZR96, CM242,
                           PU238, U234, ZR91, PU239, U235, LFP35, AM242, AM243,
                           CM244, LFP40, NP238, ZR94, PU240, AM241, LFP38, NP237,
                           LFP39, ZR92, CM247, DUMP2, CM243, U236, PU242, CM246
       Coolant             C
       Structure           MO97, SI30, NI62, W183, NI58, FE56, MN55, CR52,
                           CR53, MO92, NI60, NI61, MO98, CR50, MO96,
                           SI28, SI29, FE58, NI64, V, W184, FE54,
                           MO100, CR54, MO95, W182, MO94, FE57, W186
       ------------------  -----------------------------------------------------------
[info] Assembly zone definitions:
       --------  ------  ------------  -----------------------------------------  ----------  ------
           Zone                                                                     Symmetry  Hot
         Number  Name      Assemblies  Locations                                      Factor  Zone
       --------  ------  ------------  -----------------------------------------  ----------  ------
              1  ring-1            31  A1001, A2001, A2002, A3001, A3002, A3003,           3  False
                                       A3012, A4001, A4002, A4003, A4004,
                                       A4005, A4018, A5001, A5002, A5003,
                                       A5004, A5005, A5006, A5023, A5024,
                                       A6001, A6002, A6003, A6004, A6005,
                                       A6006, A6007, A6008, A6029, A6030
       --------  ------  ------------  -----------------------------------------  ----------  ------
[info] The setting `splitZones` is enabled. Building subzones from core zones:
       --------  ------------------  ----------  ------------  -----------  ----------  ------
           Zone                      Original                                 Symmetry  Hot
         Number  Name                Name          Assemblies  Locations        Factor  Zone
       --------  ------------------  ----------  ------------  -----------  ----------  ------
              1  ring-1-reflector-1  ring-1                 1  A4005                 3  False
       --------  ------------------  ----------  ------------  -----------  ----------  ------
```
gives some spurious messages about the assemblies. I think this also causes the vtu file to not reflect the core geometry, or what I believe the core geometry should indicate
![paraview](https://user-images.githubusercontent.com/19477741/108426248-66758680-71f0-11eb-9eeb-4ae5a642b96a.png)

As a note, the current HEAD 1fd21f8236763f43c3bc25b866122aa9789486bc produces similar issues, but with different assembly naming
```
              1  ring-1            31  001-001, 002-001, 002-002, 003-001, 003-002, 003-003,           3  False
                                       003-012, 004-001, 004-002, 004-003, 004-004,
                                       004-005, 004-018, 005-001, 005-002, 005-003,
                                       005-004, 005-005, 005-006, 005-023, 005-024,
                                       006-001, 006-002, 006-003, 006-004, 006-005,
                                       006-006, 006-007, 006-008, 006-029, 006-030
```

I've thought about not using the `lattice map` for the pin grid and instead using `grid contents` which may let me specify the pin position with tuples (ring, pos) if I'm understanding the docs right - https://terrapower.github.io/armi/.apidocs/armi.reactor.blueprints.gridBlueprint.html#armi.reactor.blueprints.gridBlueprint.GridBlueprint.gridContents

As a consistency check, if you load this reactor up, the fuel lattice appears to be present in each block with the correct lattice structure. Checked based on the `spatialLocator` for circular components in a fuel block.

Discussion:
youngmit: Ohh, thanks for keeping me honest; added a release note

Patch:
diff --git a/armi/bookkeeping/db/database3.py b/armi/bookkeeping/db/database3.py
index 10617414..3ef7891c 100644
--- a/armi/bookkeeping/db/database3.py
+++ b/armi/bookkeeping/db/database3.py
@@ -1788,10 +1788,12 @@ def _unpackLocationsV2(locationTypes, locData):
         elif lt.startswith(LOC_MULTI):
             # extract number of sublocations from e.g. "M:345" string.
             numSubLocs = int(lt.split(":")[1])
+            multiLocs = []
             for _ in range(numSubLocs):
                 subLoc = next(locsIter)
                 # All multiindexes sublocs are index locs
-                unpackedLocs.append(tuple(int(i) for i in subLoc))
+                multiLocs.append(tuple(int(i) for i in subLoc))
+            unpackedLocs.append(multiLocs)
         else:
             raise ValueError(f"Read unknown location type {lt}. Invalid DB.")
 
@@ -1842,6 +1844,14 @@ class Layout:
         self.indexInData: List[int] = []
         self.numChildren: List[int] = []
         self.locationType: List[str] = []
+        # There is a minor asymmetry here in that before writing to the DB, this is
+        # truly a flat list of tuples. However when reading, this may contain lists of
+        # tuples, which represent MI locations. This comes from the fact that we map the
+        # tuples to Location objects in Database3._compose, but map from Locations to
+        # tuples in Layout._createLayout. Ideally we would handle both directions in the
+        # same place so this can be less surprising. Resolving this would require
+        # changing the interface of the various pack/unpack functions, which have
+        # multiple versions, so the update would need to be done with care.
         self.location: List[Tuple[int, int, int]] = []
         self.gridIndex: List[int] = []
         self.temperatures: List[float] = []
diff --git a/armi/reactor/grids.py b/armi/reactor/grids.py
index 4164d83e..2f277db3 100644
--- a/armi/reactor/grids.py
+++ b/armi/reactor/grids.py
@@ -731,10 +731,20 @@ class Grid:
         for _indices, locator in self.items():
             locator._grid = self
 
-    def __getitem__(self, ijk):
+    def __getitem__(self, ijk: Union[Tuple[int, int, int], List[Tuple[int, int, int]]]):
         """
         Get a location by (i, j, k) indices. If it does not exist, create a new one and return it.
 
+        Parameters
+        ----------
+
+        ijk : tuple of indices or list of the same
+            If provided a tuple, an IndexLocation will be created (if necessary) and
+            returned. If provided a list, each element will create a new IndexLocation
+            (if necessary), and a MultiIndexLocation containing all of the passed
+            indices will be returned.
+
+
         Notes
         -----
 
@@ -745,11 +755,18 @@ class Grid:
         """
         try:
             return self._locations[ijk]
-        except KeyError:
+        except (KeyError, TypeError):
+            pass
+
+        if isinstance(ijk, tuple):
             i, j, k = ijk
             val = IndexLocation(i, j, k, self)
             self._locations[ijk] = val
-            return val
+        elif isinstance(ijk, list):
+            val = MultiIndexLocation(self)
+            locators = [self[idx] for idx in ijk]
+            val.extend(locators)
+        return val
 
     def __len__(self):
         return len(self._locations)
diff --git a/doc/release/0.1.rst b/doc/release/0.1.rst
index 9f6ba29b..e8d8938a 100644
--- a/doc/release/0.1.rst
+++ b/doc/release/0.1.rst
@@ -202,11 +202,16 @@ API changes
   objects are largely vestigial. ``SystemLayoutInput`` will be retained to facilitate
   input migrations.
 
-* Changed block default names so that they are no longer constrained by axial characters. 
+* Changed block default names so that they are no longer constrained by axial characters.
   They now are named ``B{assemNum}-{axialIndex}`` to allow arbitrary numbers of blocks. This
   will invalidate any user setting that includes a block name (e.g. detail assemblies)
 
 * Changed location string labels to be numerical (``001-002-005``) rather than alphanumeric
   to eliminate a limitation on how many i-indices and k-indices were allowed. This will
-  invalidate any user setting value that includes a location label (e.g. in 
+  invalidate any user setting value that includes a location label (e.g. in
   ``detailAssemsByBOLLocation``). A migration script may be used to assist in migration.
+
+Bug fixes
+---------
+
+ * Fix bug in loading from databases when multi-index locations are used.
diff --git a/doc/user/outputs/database.rst b/doc/user/outputs/database.rst
index 97d719a0..dc159d82 100644
--- a/doc/user/outputs/database.rst
+++ b/doc/user/outputs/database.rst
@@ -117,42 +117,42 @@ documentation of the database modules.
 
        Also, it is important to note that all components are flattened and then grouped
        by type.
-   * - ``/c{CC}n{NN}/hierarchy/``
+   * - ``/c{CC}n{NN}/layout/``
      - H5Group
      - A group that contains  a description of the ARMI model within this timenode
-   * - ``/c{CC}n{NN}/hierarchy/name``
+   * - ``/c{CC}n{NN}/layout/name``
      - list of strings
      - ``comp.name``
-   * - ``/c{CC}n{NN}/hierarchy/type``
+   * - ``/c{CC}n{NN}/layout/type``
      - list of strings
      - ``type(comp).__name__`` -- The name of the component type. We can use this to
        construct a new object when reading. You could also use it to filter down to data
        that you care about using hdf5 directly.
-   * - ``/c{CC}n{NN}/hierarchy/serialNum``
+   * - ``/c{CC}n{NN}/layout/serialNum``
      - list of int
      - ``comp.p.serialNum`` -- Serial number of the component. This number is unique
        within a component type.
-   * - ``/c{CC}n{NN}/hierarchy/location``
+   * - ``/c{CC}n{NN}/layout/location``
      - list of 3-tuple floats
      - ``tuple(comp.spatialLocator) or (0, 0, 0)`` -- Gives the location indices for a
        given component. Note these are relative, so there are duplicates.
-   * - ``/c{CC}n{NN}/hierarchy/locationType``
+   * - ``/c{CC}n{NN}/layout/locationType``
      - list of strings
      - ``type(comp.spatialLocator).__name__ or "None"`` -- The type name of the
        location.
-   * - ``/c{CC}n{NN}/hierarchy/indexInData``
+   * - ``/c{CC}n{NN}/layout/indexInData``
      - list of int
      - The components are grouped by ``type(comp).__name__``. The integers are a mapping
        between the component and its index in the ``/c{CC}n{NN}/{COMP_TYPE}/`` group.
-   * - ``/c{CC}n{NN}/hierarchy/numChildren``
+   * - ``/c{CC}n{NN}/layout/numChildren``
      - list of int
      - ``len(comp)`` -- The number of direct child composites this composite has.
        Notably, this is not a summation of all the children.
-   * - ``/c{CC}n{NN}/hierarchy/temperatures``
+   * - ``/c{CC}n{NN}/layout/temperatures``
      - list of 2-tuple floats
      - ``(comp.InputTemperatureInC, comp.TemperatureInC) or (-900, -900)`` --
        Temperatures in for Component objects.
-   * - ``/c{CC}n{NN}/hierarchy/material``
+   * - ``/c{CC}n{NN}/layout/material``
      - list of string
      - ``type(comp.material).__name__ or ""`` -- Name of the associated material for an
        Component.
@@ -165,7 +165,7 @@ documentation of the database modules.
    * - ``/c{CC}n{NN}/{COMP_TYPE}/{PARAMETER}``
      - list of inferred data
      - Values for all parameters for a specific component type, in the order defined by
-       the ``/c{CC}n{NN}/hierarchy/``. See the next table to see a description of the
+       the ``/c{CC}n{NN}/layout/``. See the next table to see a description of the
        attributes.
 
 


Test patch:
diff --git a/armi/bookkeeping/db/tests/test_database3.py b/armi/bookkeeping/db/tests/test_database3.py
index ae557158..d05d801f 100644
--- a/armi/bookkeeping/db/tests/test_database3.py
+++ b/armi/bookkeeping/db/tests/test_database3.py
@@ -321,7 +321,7 @@ class Test_LocationPacking(unittest.TestCase):
 
         self.assertEqual(unpackedData[0], (1, 2, 3))
         self.assertEqual(unpackedData[1], (4.0, 5.0, 6.0))
-        self.assertEqual(unpackedData[2], (7, 8, 9))
+        self.assertEqual(unpackedData[2], [(7, 8, 9), (10, 11, 12)])
 
 
 if __name__ == "__main__":
diff --git a/armi/reactor/tests/test_grids.py b/armi/reactor/tests/test_grids.py
index 3fe3918a..a2fae8ce 100644
--- a/armi/reactor/tests/test_grids.py
+++ b/armi/reactor/tests/test_grids.py
@@ -177,6 +177,20 @@ class TestGrid(unittest.TestCase):
         reduction = grid.reduce()
         self.assertAlmostEqual(reduction[0][1][1], 1.0)
 
+    def test_getitem(self):
+        """
+        Test that locations are created on demand, and the multi-index locations are
+        returned when necessary.
+        """
+        grid = grids.HexGrid.fromPitch(1.0, numRings=0)
+        self.assertNotIn((0, 0, 0), grid._locations)
+        loc = grid[0, 0, 0]
+        self.assertIn((0, 0, 0), grid._locations)
+
+        multiLoc = grid[[(0, 0, 0), (1, 0, 0), (0, 1, 0)]]
+        self.assertIsInstance(multiLoc, grids.MultiIndexLocation)
+        self.assertIn((1, 0, 0), grid._locations)
+
 
 class TestHexGrid(unittest.TestCase):
     def testPositions(self):
diff --git a/armi/tests/test_lwrInputs.py b/armi/tests/test_lwrInputs.py
index 91ea2bd3..149be29a 100644
--- a/armi/tests/test_lwrInputs.py
+++ b/armi/tests/test_lwrInputs.py
@@ -15,6 +15,8 @@
 import unittest
 import os
 
+import numpy
+
 from armi.utils import directoryChangers
 from armi.tests import TEST_ROOT
 from armi.reactor.flags import Flags
@@ -39,7 +41,7 @@ class C5G7ReactorTests(unittest.TestCase):
 
     def test_loadC5G7(self):
         """
-        Load the C5G7 case and check basic counts.
+        Load the C5G7 case from input and check basic counts.
         """
         o = armi.init(fName=TEST_INPUT_TITLE + ".yaml")
         b = o.r.core.getFirstBlock(Flags.MOX)
@@ -50,10 +52,25 @@ class C5G7ReactorTests(unittest.TestCase):
         gt = b.getComponent(Flags.GUIDE_TUBE)
         self.assertEqual(gt.getDimension("mult"), 24)
 
-    def test_runC5G7(self):
+    def test_runAndLoadC5G7(self):
         """
-        Run C5G7 in basic no-op app and load from the result.
+        Run C5G7 in basic no-op app and load from the result from DB.
+
+        This ensures that these kinds of cases can be read from DB.
         """
+
+        def _loadLocs(o, locs):
+            for b in o.r.core.getBlocks():
+                indices = b.spatialLocator.getCompleteIndices()
+                locs[indices] = b.spatialLocator.getGlobalCoordinates()
+
         o = armi.init(fName=TEST_INPUT_TITLE + ".yaml")
+        locsInput, locsDB = {}, {}
+        _loadLocs(o, locsInput)
         o.operate()
         o2 = db.loadOperator(TEST_INPUT_TITLE + ".h5", 0, 0)
+        _loadLocs(o2, locsDB)
+
+        for indices, coordsInput in sorted(locsInput.items()):
+            coordsDB = locsDB[indices]
+            self.assertTrue(numpy.allclose(coordsInput, coordsDB))