Fix for rotating bounding box the wrong way #199
Conversation
facebook-github-bot
added
the
CLA Signed
|
Hi @juliusfrost, thanks for this contribution and surfacing that you've seen unexpected behavior with bounding boxes with the rotate augmentation! I double checked the PIL source code and the rotation matrix which we currently create in this helper function does align with the rotation matrix used by PIL here. Can you please prove that there was indeed an issue with the way the calculation is done now (i.e. run a test case & visualize it where the bbox is rotated incorrectly), and show that that case is fixed by this fix? I want to be careful making changes since this helper function is fairly complex. Thanks! |
|
@zpapakipos I provided a test case on a separate branch here: https://github.com/juliusfrost/AugLy/blob/rotate-bounding-box-jupyter/examples/test_rotate.ipynb |
|
@zpapakipos has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@juliusfrost thank you for the notebook with the thorough test cases! :) Looks good to me; can you just rebase onto the latest main changes to fix the linter error? Then we should be good to go, thanks! |
|
Hi @juliusfrost, reminder to please rebase onto main once #204 has been merged :) (should be within a few minutes) |
|
@juliusfrost has updated the pull request. You must reimport the pull request before landing. |
2 similar comments
|
@juliusfrost has updated the pull request. You must reimport the pull request before landing. |
|
@juliusfrost has updated the pull request. You must reimport the pull request before landing. |
|
@juliusfrost has updated the pull request. You must reimport the pull request before landing. |
|
@zpapakipos I think it's ready now |
|
@zpapakipos has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: - [x] I have read CONTRIBUTING.md to understand how to contribute to this repository :) The current method for rotating bounding boxes was bugged because it rotated the bounding box in the wrong direction. This simple sign change fixes it. Test case: https://github.com/juliusfrost/AugLy/blob/rotate-bounding-box-jupyter/examples/test_rotate.ipynb ## Unit Tests Passes unit tests. Pull Request resolved: facebookresearch#199 Reviewed By: jbitton Differential Revision: D34813315 Pulled By: zpapakipos fbshipit-source-id: e1212d2a95a4837ebdc9ff3e97edebb907b93988
Summary
The current method for rotating bounding boxes was bugged because it rotated the bounding box in the wrong direction. This simple sign change fixes it.
Test case: https://github.com/juliusfrost/AugLy/blob/rotate-bounding-box-jupyter/examples/test_rotate.ipynb
Unit Tests
Passes unit tests.