Fix order of operation bug in rotatePins. #288
Conversation
RotatePins uses modulus (%) to avoid over-rotating, but the order of operations was wrong: initialRotation + newRotation % 6 instead of (initialRotation + newRotation) % 6 In some cases, this could lead to creation of pin index numbers that were larger than the number of pins.
|
@mgjarrett, Can you add a unit test that fails with the previous implementation and that also demonstrates that this fixes the rotation behavior? |
If the sum of the current rotation state and the additional rotation exceeds 6, the old rotatePins function would over-rotate. The unit test was modified to induce this situation, where a 4 x 60 degree rotation is applied to a block that has already been rotated 4 x 60. In this case, the old rotatePins would calculate a rotNum of 8, which would make some pin indexes incorrect.
|
I added a unit test that would fail with the previous implementation. I don't know why black is failing. It seems unhappy with some docstrings in files that I didn't touch. I checked and it seems like I'm running the same version of black, but when I run black on these files manually it does nothing. So I'm kind of perplexed. |
|
@mgjarrett make sure you have the latest pip and then try to update black. It seems that this CI is using an updated version. |
|
@jakehader I was using 20.8b1, but I think this is out of my control anyway. The affected files are runLog.py, reactors.py, database3.py, mainInterface.py, a bunch of materials files, etc. These files are not modified in the pull request. |
|
Black's GH action is kind of busted in that when you request a specific version, it still uses whatever the latest happens to be. We already merged a PR with different formatting so I'd say we just merge this. There is some ongoing discussion here: psf/black#1939 and here: psf/black#1940 |
|
That explains my confusion. it looks like Jake has the formatting changes covered in #289. |
RotatePins uses modulus (%) to avoid over-rotating, but the order of
operations was wrong:
initialRotation + newRotation % 6
instead of
(initialRotation + newRotation) % 6
In some cases, this could lead to creation of pin index numbers that
were larger than the number of pins.