Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

plotBlockDiagram() doesn't respect block orientation #1421

Open
keckler opened this issue Oct 2, 2023 · 5 comments · May be fixed by #1648
Open

plotBlockDiagram() doesn't respect block orientation #1421

keckler opened this issue Oct 2, 2023 · 5 comments · May be fixed by #1648
Labels
bug Something is wrong: Highest Priority complex Expected to be a complex issue

Comments

@keckler
Copy link
Member

keckler commented Oct 2, 2023

The plotBlockDiagram() function appears to always assume that the lattice within the block is "flats-up". This becomes an issue if one has manually defined a "points-up"
pin grid. In that case you'll get a print like the following:
image

The problem lines appear to be in this chunk:

armi/armi/utils/plotting.py

Lines 1283 to 1290 in 8041ad8

elif isinstance(component, Hexagon):
if component.getDimension("ip", cold=cold) != 0:
innerPoints = numpy.array(
hexagon.corners(30) * component.getDimension("ip", cold=cold)
)
outerPoints = numpy.array(
hexagon.corners(30) * component.getDimension("op", cold=cold)
)

Instead of always rotating the hex patch by 30 degrees, it needs to instead be able to interrogate the underlying grid to know the rotation. Unfortunately I think this is a challenge right now, as the block itself doesn't know the type of grid that is in it (that info is stored on the associated block blueprint). Even still, many block blueprints don't actually have a grid attribute even if the block later gets an auto-generated grid. This is related to #730 (comment)

This type of problem could be solved once #1284 is implemented, I would guess.
@keckler keckler added bug Something is wrong: Highest Priority complex Expected to be a complex issue low priority labels Oct 5, 2023
@drewj-usnctech
Copy link
Contributor

Possibly related to a lot of the assumption that most hexagons are flats up - #1278

@mgjarrett
Copy link
Contributor

I don't think the block needs to know what kind of grid is inside it, it just needs to know what kind of grid it is in. For a hex grid, the block grid and the pin lattice should be opposites. If they're not opposites, that's an input error.

@keckler
Copy link
Member Author

keckler commented Feb 15, 2024

I don't think the block needs to know what kind of grid is inside it, it just needs to know what kind of grid it is in. For a hex grid, the block grid and the pin lattice should be opposites. If they're not opposites, that's an input error.

I agree with this idea, and it is actually pretty slick.

However, if you consider the printed diagrams to be a tool for diagnosing input errors, then maybe relying on the user being correct is not a good idea. Unless, of course, there is another check in place to ensure that the grid and block rotations are appropriate for each other.

@mgjarrett mgjarrett linked a pull request Feb 15, 2024 that will close this issue
Draft
8 tasks
@mgjarrett
Copy link
Contributor

I don't think the block needs to know what kind of grid is inside it, it just needs to know what kind of grid it is in. For a hex grid, the block grid and the pin lattice should be opposites. If they're not opposites, that's an input error.

I agree with this idea, and it is actually pretty slick.

However, if you consider the printed diagrams to be a tool for diagnosing input errors, then maybe relying on the user being correct is not a good idea. Unless, of course, there is another check in place to ensure that the grid and block rotations are appropriate for each other.

I agree that there should be input validation to ensure the blocks and pins are correctly rotated w.r.t. one another. We don't currently have any such input validation that I know of.

@keckler
Copy link
Member Author

keckler commented Feb 25, 2024

See #1648 for an incoming fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something is wrong: Highest Priority complex Expected to be a complex issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix rotation of hexagons in block diagrams mgjarrett/armi
3 participants
@mgjarrett @keckler @drewj-usnctech