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.
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
Fix the hex orientation of pin locations. #730
Conversation
3b19d60
to
656bce4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think also that your black
tests are failing.
# note that it's the pointed end of the cell hexes that are up (but the | ||
# macro shape of the pins forms a hex with a flat top fitting in the assembly) | ||
grid = grids.HexGrid.fromPitch( | ||
self.getPinPitch(cold=True), numRings=0, pointedEndUp=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still working if a user puts into their layout blueprint the geom: hex_corners_up
option?
I can't quite tell myself, but it feels like maybe in that case you should set pointedEndUp=False
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it only works on flat side up cases, you're right. I added a note in the docstring to the effect. It's be nice to detect that situation and throw an error, but we don't currently have a good way to look at a grid and determine which 'orientation' it is at this level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't currently have a good way to look at a grid and determine which 'orientation'
Sad trombone noises.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found this researching #1284
@ntouran |
The pin locations in a hex block were rotated 30 degrees from where they should be. This was discovered when plotting pin-level assembly plots and they were all sticking out the edges. The major confusion comes in from the fact that even though pointed end up doesn't seem right when you look at the pins, if you look at the little outline hexes surrounding each pin then you can see that indeed the pointed end is supposed to be up in this case. This could impact results for anyone using auto-generated pin-level grids and using the pin-wise pin coordinates.
After fixing the bug, I noticed that this other method was basically duplicated and out of date. I updated it to be consistent with the spatialLocators.
Fix formatting.
32bd1e5
to
951c1ee
Compare
The pin locations in a hex block were rotated 30 degrees from where they should be. This was discovered when plotting pin-level assembly plots and they were all sticking out the edges. The major confusion comes in from the fact that even though pointed end up doesn't seem right when you look at the pins, if you look at the little outline hexes surrounding each pin then you can see that indeed the pointed end is supposed to be up in this case. This could impact results for anyone using auto-generated pin-level grids and using the pin-wise pin coordinates.
The pin locations in a hex block were rotated 30 degrees from where they
should be. This was discovered when plotting pin-level assembly plots
and they were all sticking out the edges. The major confusion comes in
from the fact that even though pointed end up doesn't seem right when
you look at the pins, if you look at the little outline hexes
surrounding each pin then you can see that indeed the pointed end is
supposed to be up in this case.
This could impact results for anyone using auto-generated pin-level
grids and using the pin-wise pin coordinates.
Before:
After:
Checklist
If user exposed functionality was added/changed:
doc
folder.