armi icon indicating copy to clipboard operation
armi copied to clipboard

Fixing HexBlock rotation in plotBlockDiagram

Open john-science opened this issue 1 year ago • 4 comments

What is the change?

  1. Fixed a bug in plotBlockDiagram().
  2. To help, I added the method HexBlock.cornersUp(), as I imagine it will be a helpful utility to have.
  3. Oh, and sorry, I changed a variable from self.HexBlock to self.hexBlock, to match our variable naming rules.

Why is the change being made?

The method plotBlockDiagram() incorrectly plots HexBlocks if they are rotated to the "corners up" rotation.

close #1421


Checklist

  • [x] The release notes have been updated if necessary.
  • [x] The documentation is still up-to-date in the doc folder.
  • [x] The dependencies are still up-to-date in pyproject.toml.

john-science avatar Oct 04 '24 16:10 john-science

First question without even looking at the code -- Can we get a picture of the correct plots?

keckler avatar Oct 04 '24 16:10 keckler

@john-science Thank you for fixing those few comments. Please also note that there are a couple more things that would be good before approving this PR:

  1. https://github.com/terrapower/armi/pull/1926#pullrequestreview-2348839742
  2. https://github.com/terrapower/armi/pull/1926#issuecomment-2394104416

keckler avatar Oct 05 '24 15:10 keckler

This PR is temporarily on hold for another PR: https://github.com/terrapower/armi/pull/1947

I believe my solution in this was over-engineered to work AROUND the problem present in the other PR. Once that other PR merges, I should be able to simplify this PR.

john-science avatar Oct 11 '24 14:10 john-science

@keckler and @drewj-tp

Here I plot armiRunSmallest.yaml with the default hex_corners_up:

hex_corners_up

and here I do the same thing, but alter setting the reactor to be "flats up":

hex_flats_up

So, after the recent bug fix, I think we finally have success!

Whew.

john-science avatar Oct 30 '24 17:10 john-science