facebookresearch / facebookresearch/detectron2

Orientation/symmetry issues in Box2BoxTransformRotated.get_deltas

Open
#1,172 8 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Python
Stars
34.7k
Forks
7.9k
PR merge metrics
No merged PRs in 30d

Description

I believe I have identified two issues with Box2BoxTransformRotated that are related to notions of orientation and symmetry.

During training, boxes are matched based on their pairwise rotated iou, however the target regression deltas can end up large with the current logic.

I illustrate my point with the following test cases:

```python
import torch

from detectron2.structures import RotatedBoxes, pairwise_iou_rotated
from detectron2.modeling.box_regression import Box2BoxTransformRotated

box2box_transform = Box2BoxTransformRotated(weights=(1, 1, 1, 1, 1))

# Failure 1

# These are the same boxes, just flipped
box1 = RotatedBoxes(torch.tensor([[40.0, 40.0, 40.0, 5.0, 0]]))
box2 = RotatedBoxes(torch.tensor([[40.0, 40.0, 40.0, 5.0, 180]]))

# tensor([[1.]])
pairwise_iou_rotated(box1, box2)

# tensor([[ 0.0000, 0.0000, 0.0000, 0.0000, -3.1416]])
box2box_transform.get_deltas(box1.tensor, box2.tensor)

# Failure 2

# These boxes lie on top of each other, just transposed
box1 = RotatedBoxes(torch.tensor([[40.0, 40.0, 40.0, 5.0, 0]]))
box2 = RotatedBoxes(torch.tensor([[40.0, 40.0, 5.0, 40.0, 90]]))

# tensor([[1.]])
pairwise_iou_rotated(box1, box2)

# tensor([[ 0.0000, 0.0000, -2.0794, 2.0794, 1.5708]])
box2box_transform.get_deltas(box1.tensor, box2.tensor)
```

In both of the above failure cases, based on iou box1 would be matched to box2 as a perfect match, but the target regression deltas are extremely large. I have seen this negatively impact training.

**Failure 1**

If we do not consider a bounding box to have an orientation, then a rotation of -110 degrees is the same as a rotation of +70 degrees. If you do consider it to have an orientation, then the +70 degree rotated box is "upside down" and incorrect.

The current Box2BoxTransformRotated.get_deltas logic maps angles to [180, 180). I assert this interprets boxes to have a orientation. The orientation invariant version of the logic would be the same, except angles would be mapped to [90, 90).

**Failure 2**

If we do not consider boxes to have an orientation, then the delta between these boxes is zero, but if we do not, then we must rotate the box 90 degrees and dramatically change its width and height.

I would propose that Box2BoxTransformRotated.get_deltas should consider two possibilities:
1. That the source width, and height should be matched to the target width and height
2. That the source width should be mapped to the target height, and vice versa with a rotation of 90 degrees.

Whichever delta is "smaller" should be considered the target. This is ambiguous because it depends on how you are inclined to trade off changes in width, height, and angle. I would propose to use the weights and the smooth L1 loss to inform this.

## Bigger picture

I think rotated bounding boxes are an incredibly valuable, but I see the complexity that they bring. Here this analysis is leading to greater complexity. I believe that there are ultimately two types of rotated bounding box. One with orientation and one without where that is ultimately defined by the ground truth labels and the use case.

Consider labelling people:
- Suppose that I always draw a rotated bounding box such that height is along the long axis of the human body, and the "up" end of the box is always the head. Suppose I also want oriented bounding box detections that align with this definition. Then I would like the entire detectron2 pipeline to respect that.
- Suppose instead I draw my bounding boxes without these considerations and I do not care about oriented detections. Then I want the entire pipeline to ignore orientation.

Some thorough analysis of this already exists in the project here:
https://github.com/facebookresearch/detectron2/blob/b8f4eebeb06827d49e31738c0997f919dac26aba/detectron2/layers/nms.py#L39

I would propose that as the vast majority of the logic is currently orientation-free and that Box2BoxTransformRotated.get_deltas should be changed accordingly.

I have a PR in development that I would be happy to raise after discussion of this issue.

In the long term, perhaps rotated bounding boxes should be split into the two types if the need arises.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at Box2BoxTransformRotated.get_deltas and reproduce the two examples using the rotated IoU behavior discussed in detectron2/layers/nms.py. The issue needs an agreed orientation policy and a comparison of the two candidate deltas using the configured weights and Smooth L1 loss; done means the behavior and regression tests are settled for both symmetry cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
computer-vision
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.