Refactor armi.reactor.grids to wider submodule; Grids as ABC

[drewj-usnctech]

What is the change?

Mainly two changes here, one largely invisible to users/developers, one maybe controversial (in a312849)

Refactor

Refactoring armi/reactor/grids.py into "wider" submodule armi.reactor.grids. Dedicated files for specific grid types, e.g., armi.reactor.grids.hexagonal provides HexGrid. All classes and utilities are imported into armi/reactor/grids/__init__.py so there should be no API-breaking changes up to a312849.

I tried to keep the commits small so you could see the process was essentially

• git mv armi/reactor/grids.py armi/reactor/grids/__init__.py
• copy some class from armi/reactor/grids/__init__.py to new file armi/reactor/grids/class.py
• reimport class from armi/reactor/grids/class.py into armi/reactor/grids/__init__.py so there are no API changes

In this process, I also added an AxialGrid class, the return type from the (proposed deprecated) armi.reactor.grids.axialUnitGrid function.

Abstraction

Introduced in a312849

Heavier lift, but the classes defined in armi.reactor.gridsfollow this inheritance pattern

• Grid(abc.ABC) :: abstract interface for a grid, or things that reside in
  space
  • StructuredGrid(Grid) :: still abstract, but for things that have
    some structure, known to ARMI as unit steps and bounds
    • HexGrid(StructuredGrid)
    • CartesianGrid(StructuredGrid)
    • ThetaRZGrid(StructuredGrid) - though how "structured" is this
      really?
    • AxialGrid(StructuredGrid)

In some cases, the shared interface between structured grids falls
apart. For example, HexGrid.pitch returns a single float, while
CartesianGrid.pitch is a 2-ple of float, one for x and one for y.
And the concept of ring and position appears to be stressed since the
inception of cartesian grids. But parts of ARMI require an interface
and so an attempt was made to define that interface.

The decision was made to not have StructuredGrid the base class, and
to introduce Grid as the base intentionally. There are cases for some
multiphysics codes where a grid may still be "structured" but not with
the level of structure the previous Grid implementations expect. One
example would be an irregular cartesian grid, where the unit cell
spacing in one or more dimensions may change. So the cell spacing is a
function of the cell position in that dimension.

This is more a breaking change because classes that used to inherit from armi.reactor.grids.Grid may not have the same unit stepping / bounds behavior they would get instead by subclassing the new armi.reactor.grids.StructuredGrid

Why is the change being made?

Primary motivation is #1278 : fixing orientation of tips up hex grids. My proposed plan, introduced in this comment is to have HexGrid be "orientation agnostic" and instead provide two subclasses, e.g., FlatsUpHexGrid and TipsUpHexGrid.

Looking through the code, it seems the spatial locators attached to these components have correct (i, j) indices. But the problem falls through HexGrid.getRingPos as that seems (after minor investigation) to assume a flats-up orientation.

Looking through more of the HexGrid implementation, making HexGrid.getRingPos more aware of the orientation seems to be the tip of the iceberg. It seems like multiple methods make this assumption

This also helps clients understand more simply the hex grid orientation, something that isn't really feasible now (see this comment)

Checklist

• This PR has only one purpose or idea.
  • maybe 1.5 ideas? it's a refactor, but like a refactor+
• Tests have been added/updated to verify that the new/changed code works.

• The code style follows good practices.
• The commit message(s) follow good practices.

• The release notes (location doc/release/0.X.rst) are up-to-date with any important changes.
  • TBD depending on acceptance of ABC grid. Otherwise it's relatively transparent from an API perspective
• The documentation is still up-to-date in the doc folder.
• The dependencies are still up-to-date in setup.py.

[drewj-usnctech]

ping @keckler @john-science @ntouran for insight / review. This is a first step in closing #1278

[john-science]

@drewj-usnctech Awesome! I love it. I have a couple of little cleanup things above, then we can merge this.

[drewj-usnctech]

Some weird pip behavior that feels like a github / docker / larger issue?

 .pkg: _exit> python /opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  cov2: FAIL code 1 (1221.10=setup[62.95]+cmd[1158.15] seconds)

[john-science]

Some weird pip behavior that feels like a github / docker / larger issue?

 .pkg: _exit> python /opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  cov2: FAIL code 1 (1221.10=setup[62.95]+cmd[1158.15] seconds)

@opotowsky Is this a Python 3.11 issue?

[opotowsky]

Some weird pip behavior that feels like a github / docker / larger issue?

 .pkg: _exit> python /opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  cov2: FAIL code 1 (1221.10=setup[62.95]+cmd[1158.15] seconds)

@opotowsky Is this a Python 3.11 issue?

I'm seeing an actual failure, actually

But why in only python 3.11?

[john-science]

@drewj-usnctech OR this might not be Python 3.11, this might just be a plain unit test failure:

https://github.com/terrapower/armi/actions/runs/5965597782/job/16183385153?pr=1373

  File "/home/runner/work/armi/armi/armi/reactor/grids/tests/test_grids.py", line 216, in test_ringPosFromIndicesIncorrect
    grid = grids.Grid(unitSteps=((1.0, 0.0, 0.0), (0.0, 1.0, 0.0), (0.0, 0.0, 1.0)))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Can't instantiate abstract class Grid with abstract methods __len__, backUp, getCellBase, getCellTop, getCoordinates, getSymmetricEquivalents, isAxialOnly, items, locatorInDomain, overlapsWhichSymmetryLine, reduce, restoreBackup

MAYBE this is a problem by in Python 3.11 they are more hard-enforcing that you must fully implement all abstract methods in an abstract class now? And it was more soft "maybe" before?

I bet it's something like that. Though the test did fail. I don't see any changes to abc in the release notes.

[opotowsky]

oh! the other tests were cancelled! prob not a py 311 problem, phew!

[drewj-usnctech]

Nope that's on me. I just missed that grids.Grid in the tests 🤦

[john-science]

I love a good refactor!

[john-science]

@drewj-usnctech It looks like the problem above that broke the unit tests also broke the docs:

/home/runner/work/armi/armi/doc/gallery-src/framework/run_grids2_cartesian.py failed leaving traceback:
Traceback (most recent call last):
  File "/home/runner/work/armi/armi/doc/gallery-src/framework/run_grids2_cartesian.py", line 34, in <module>
    cartesian_grid = grids.Grid(
TypeError: Can't instantiate abstract class Grid with abstract methods __len__, backUp, getCellBase, getCellTop, getCoordinates, getSymmetricEquivalents, isAxialOnly, items, locatorInDomain, overlapsWhichSymmetryLine, reduce, restoreBackup

CI is currently broken in ARMI.