-
Notifications
You must be signed in to change notification settings - Fork 7.1k
More rotated bboxes transforms #9095
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
More rotated bboxes transforms #9095
Conversation
Test Plan: ```bash pytest test/test_transforms_v2.py -vvv -k "TestResize and test_kernel_bounding_boxes" pytest test/test_transforms_v2.py -vvv -k "TestResize and test_bounding_boxes_correctness" ````
Test Plan: Run unit tests: ```bash pytest test/test_transforms_v2.py -vvv -k "TestAffine and test_kernel_bounding_boxes" pytest test/test_transforms_v2.py -vvv -k "TestAffine and test_functional_bounding_boxes_correctness" ```
Test Plan: ```bash pytest test/test_transforms_v2.py -vvv -k "TestPad and test_kernel_bounding_boxes" pytest test/test_transforms_v2.py -vvv -k "TestPad and test_bounding_boxes_correctness" ```
Test Plan: ```bash pytest test/test_transforms_v2.py -vvv -k "TestCrop and test_kernel_bounding_box" pytest test/test_transforms_v2.py -vvv -k "TestCrop and test_functional_bounding_box_correctness" pytest test/test_transforms_v2.py -vvv -k "TestCrop and test_transform_bounding_boxes_correctness" pytest test/test_transforms_v2.py -vvv -k "TestCenterCrop and test_kernel_bounding_boxes" pytest test/test_transforms_v2.py -vvv -k "TestCenterCrop and test_bounding_boxes_correctness" pytest test/test_transforms_v2.py -vvv -k "TestResizedCrop and test_functional_bounding_boxes_correctness" ```
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9095
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 2f322f0 with merge base 428a54c ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
Thanks @AntoineSimoulin , I have a few comments and questions but this looks good!
@@ -560,7 +560,9 @@ def affine_bounding_boxes(bounding_boxes): | |||
) | |||
|
|||
|
|||
def reference_affine_rotated_bounding_boxes_helper(bounding_boxes, *, affine_matrix, new_canvas_size=None, clamp=True): | |||
def reference_affine_rotated_bounding_boxes_helper( | |||
bounding_boxes, *, affine_matrix, new_canvas_size=None, clamp=True, flip=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.
Can you help me understand why we need a flip
parameter in reference_affine_rotated_bounding_boxes_helper
, but not in reference_affine_bounding_boxes_helper
?
It seems that reference_affine_bounding_boxes_helper
, the flip is done through the affine_matrix
? Is this something we can do for the rotated case as well? If not it might be worth adding a comment to explain why
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.
@NicolasHug, this is a good question. As illustrated in the image below. When we apply the flip operation, we actually transform each points (x1, y1), (x2, y2), (x3, y3), (x4, y4) from the bounding box. However the flip operation will typically change the order of the points if we go through them in clock-wise order. Given the definition of "XYXYXYXY", "CXCYWHR", and "XYWHR", we do want to go through (x1, y1), (x2, y2), (x3, y3), (x4, y4) in clock-wise order. This is why we are switching the point (x2, y2) and (x4, y4). This typically does not append for non-rotated boxes as we re-assign the points with min
and max
operations here. I hope it makes sense?
Test Plan: Run unit tests ```bash pytest test/test_transforms_v2.py -vvv -k "test_parallelogram_to_bounding_boxes" ```
Thanks for the in-depth review @NicolasHug. I submitted a few commits which should answer all your comments. Here is a list of the modifications I made to address your comments:
|
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.
Thanks a lot @AntoineSimoulin !
Hey @NicolasHug! You merged this PR, but no labels were added. |
Add Transforms support for Rotated Boxes
This PR is the first of a series to add support for rotated boxes transforms. In particular this PR implements the following functionalities:
is_rotated_bounding_box_format
in "/transforms/v2/functional/_meta.py" to rely on the utility functiontv_tensors.is_rotated_bounding_format
introduced in Rotated bboxes transforms #9084;_parallelogram_to_bounding_boxes
to adjust the output box in such cases (c.f. illustration below);Figure 1. Illustration of transformation introducing non-parallelism for rotated boxes and of how the
_parallelogram_to_bounding_boxes
reconstruct a rotated box from a parallelogram shape.Test plan
Please run the following tests:
Questions
Please note that this PR introduces a change in the API signature of the
resize_bounding_boxes
function to addformat: tv_tensors.BoundingBoxFormat
and specialize the operation given whether the box is rotated or not. Contrary to other transforms, this function did typically not received theformat
parameter as input before. Is it possible to add it here? Should we maybe add a default value in this case to preserve the behavior for non-rotated box by default?Future work
This PR implements only 4 transforms for rotated boxes. We plan to release the remaining 3 transforms in a subsequent PR.