Rotated bboxes transforms#9084
Conversation
Test Plan: Run unit tests:`pytest test/test_tv_tensors.py -vvv -k "test_bbox_format"`
Test Plan: Run unit tests: `pytest test/test_transforms_v2.py -vvv -k "TestHorizontalFlip and test_kernel_bounding_boxes"` and `pytest test/test_transforms_v2.py -vvv -k "TestHorizontalFlip and test_bounding_boxes_correctness"`
Test Plan: Run unit tests: `pytest test/test_transforms_v2.py -vvv -k "TestVerticalFlip and test_kernel_bounding_boxes"`
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9084
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
NicolasHug
left a comment
There was a problem hiding this comment.
Thanks for the PR @AntoineSimoulin , I made a first round of comments
| elif image.size(0) not in {1, 3}: | ||
| raise ValueError("Only grayscale and RGB images are supported") | ||
| elif (boxes[:, 0] > boxes[:, 2]).any() or (boxes[:, 1] > boxes[:, 3]).any(): | ||
| elif boxes.shape[-1] == 4 and ((boxes[:, 0] > boxes[:, 2]).any() or (boxes[:, 1] > boxes[:, 3]).any()): |
There was a problem hiding this comment.
Nit: since we seem to use both boxes.shape[-1] == 4 and len(bbox) == 4 as condition checks, it might help to create a unified boolean variable e.g.
is_rotated = boxes.shape[-1] == 4and use is_rotated for the remainder of the function. It also makes the conditions slightly more explicit about what they're checking against.
There was a problem hiding this comment.
I would prefer not to create a variable before the testing at the beginning of the function. But I simplified the core of the function given your other comments!
| # operation | ||
| dtype = output.dtype | ||
|
|
||
| return output.to(dtype=dtype, device=device) |
There was a problem hiding this comment.
Shouldn't we cast back to the input dtype unconditionally? In general the transforms should preserve the input dtype, but here's it's not clear that we are?
There was a problem hiding this comment.
In this test, we are creating an intermediate tensor and therefore make sure to cast it to the correct dtype.
AntoineSimoulin
left a comment
There was a problem hiding this comment.
@NicolasHug thanks for reviewing! I should have addressed all comments here. Additional tests for visualization can be run with pytest test/test_utils.py -vvv -k "test_draw_rotatated_boxes". Let me know if you have additional comment to add in this review.
| # operation | ||
| dtype = output.dtype | ||
|
|
||
| return output.to(dtype=dtype, device=device) |
There was a problem hiding this comment.
In this test, we are creating an intermediate tensor and therefore make sure to cast it to the correct dtype.
|
|
||
| # TODO: Once torchscript supports Enums with staticmethod | ||
| # this can be put into BoundingBoxFormat as staticmethod | ||
| def is_rotated_bounding_format(format: BoundingBoxFormat) -> bool: |
There was a problem hiding this comment.
Hey @NicolasHug, I tried to do that. Unfortunately it is not compatible with TorchScript. Indeed BoundingBoxes is directly inheriting from torch.Tensor and TorchScript does not fully support inheritance from built-in PyTorch types like torch.Tensor and has specific rules and limitations regarding which methods and attributes are accessible when scripting custom classes that inherit from these types.
| boxes (Tensor): Tensor of size (N, 4) containing bounding boxes in (xmin, ymin, xmax, ymax) format. Note that | ||
| the boxes are absolute coordinates with respect to the image. In other words: `0 <= xmin < xmax < W` and | ||
| `0 <= ymin < ymax < H`. | ||
| boxes (Tensor): Tensor of size (N, 4) or (N, 8) containing bounding boxes. |
There was a problem hiding this comment.
This sounds good. I propose we do address this in a subsequent PR!
| elif image.size(0) not in {1, 3}: | ||
| raise ValueError("Only grayscale and RGB images are supported") | ||
| elif (boxes[:, 0] > boxes[:, 2]).any() or (boxes[:, 1] > boxes[:, 3]).any(): | ||
| elif boxes.shape[-1] == 4 and ((boxes[:, 0] > boxes[:, 2]).any() or (boxes[:, 1] > boxes[:, 3]).any()): |
There was a problem hiding this comment.
I would prefer not to create a variable before the testing at the beginning of the function. But I simplified the core of the function given your other comments!
NicolasHug
left a comment
There was a problem hiding this comment.
Thank you @AntoineSimoulin !
|
Hey @AntoineSimoulin! You merged this PR, but no labels were added. |
Summary: Co-authored-by: Nicolas Hug <contact@nicolas-hug.com> Reviewed By: AntoineSimoulin Differential Revision: D79175063 fbshipit-source-id: 7389510a020fae499cfe8812a0d26d9adb90c744
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:
draw_bounding_boxesto acceptBoundingBoxeswith formatBoundingBoxFormat.XYXYXYXY;is_rotated_bounding_formatto infer whether aBoundingBoxFormatcorresponds to a rotated format or not. To preserve torchscript support, this function is not added as a method from theEnumdefinition ofBoundingBoxFormat;Test plan
Please run the following tests:
Plot function
The plot function can be used as follow
import torch from gallery.transforms.helpers import plot from test.common_utils import make_bounding_boxes from torchvision import transforms, tv_tensors img = torch.ones(3, 360, 360) boxes = make_bounding_boxes( canvas_size=(360, 360), format=tv_tensors.BoundingBoxFormat.XYXYXYXY, num_boxes=2, ) plot([(img, boxes)])Future work
This PR implements only two transforms for rotated boxes. However, it is intended to validate the implementation before releasing other transforms. Please also note that the clamping transforms is for now just a placeholder to make sure we do pass the tests for rotated boxes.
cc @vfdev-5