Adds Generalized IOU, Tests for Box Ops

[oke-aditya]

Fixes #2545

As discussed there, Generalized IOU would be a good feature to add.
I have added that.
The reference implementation is from detr as suggested by fmassa.

Detr has a slightly different implementation of box_iou from torchvision which is used by generalized_iou.
We return union and iou both in detr, whereas we return only iou in torchvision.

I thought not to change the API and instead hence did NOT re-use box_iou in generalized_iou.
This ensures both these functions would return a single tensor parameter.

I did a bit of code duplication which might not be best practice, please let me know if that can be avoided.
I had thought of making a helper function that returns both iou and union from which we choose in both cases.

• Adds Generalized IoU implementation.
• Add Tests.
• Add docs.

For adding tests, I could not find any class in test_ops.py file for IOU. I could see for NMS etc, I'm unsure where IoU tests are located.

Maybe I overlooked or looked at the wrong place. Do let me know where and how the tests should be!

For docs I saw torchvision ops.rst file. But I cannot find docs for IoU. I'm unsure where to add docs.

Let me know where are they needed.

cc: @fmassa @pmeier

[pmeier]

@oke-aditya I'm unsubscribing for now. Ping me again when this is ready for review, i.e. not a draft anymore.

[oke-aditya]

Docs are not added for some of the boxes.py codebase in ops.rst file.

I added docs for iou and as well as generalized_iou. Since they both use box_area I added it too.

Tested these docs locally.

[oke-aditya]

For tests, I thought to keep it similar to IoU tests. But I cannot find tests for IoU and I am unsure how and where to add it.
Please let me know if this needs changes.
cc @pmeier

[pmeier]

Hey @oke-aditya, I'm pretty covered right now, so reviewing this will take a while. Sorry for the delay.

[oke-aditya]

No issues @pmeier 😀

[codecov]

Codecov Report

Merging #2642 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2642      +/-   ##
==========================================
+ Coverage   72.82%   72.87%   +0.05%     
==========================================
  Files          95       95              
  Lines        8212     8228      +16     
  Branches     1283     1283              
==========================================
+ Hits         5980     5996      +16     
  Misses       1838     1838              
  Partials      394      394              

Impacted Files	Coverage Δ
torchvision/ops/__init__.py	100.00% <100.00%> (ø)
torchvision/ops/boxes.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15848ed...29c11ac. Read the comment docs.

[oke-aditya]

I had to fix this PR due to Merge conflict of my previous PR. Linter failed here. I will fix it.

I hope this goes into the next release.

We need tests for IoU, gIoU and other box operations. Do let me know if they are needed. Maybe I can add in a separate PR.

./torchvision/ops/__init__.py:18:49: E231 missing whitespace after ','

[fmassa]

Hi @oke-aditya

Thanks for the PR and sorry for the delay in reviewing!

The way you did it (without re-using box_iou) seems ok to me.
For the docs I think everything is good now!

For tests, I propose that we cover a couple of known cases, like Generalized IoU between boxes which are the same, boxes that do not overlap at all, etc (like, at least the sign of the boxes should be known).

We can also take the result from the DETR implementation for a couple of pre-defined boxes and compare against our implementation here.

[oke-aditya]

I added tests for box_iou generalized_iou and box_area. I covered some cases where IoU is 0, 1 and 0.25 .

For giou they are inf, 1 and 3.25 . Please let me know if they right.

[fmassa]

Thanks for the tests, they look great!

I think there is a bug with the implementation, let me know what you think

[oke-aditya]

Ok, so there is small torch issue why the tests are failing. I updated and seem to get correct result as above.

boxes1 = torch.tensor([[0, 0, 100, 100], [0, 0, 50, 50], [200, 200, 300, 300]], dtype=torch.float)
boxes2 = torch.tensor([[0, 0, 100, 100], [0, 0, 50, 50], [200, 200, 300, 300]], dtype=torch.float)
out = generalized_box_iou(boxes1, boxes2)
expected = torch.tensor([[1.0, 0.25, -0.7778], [0.25, 1.0, -0.8611], [-0.7778, -0.8611, 1.0]], dtype=torch.float)

The issue is out is

tensor([[ 1.0000,  0.2500, -0.7778],
[ 0.2500,  1.0000, -0.8611],
[-0.7778, -0.8611,  1.0000]]) 

where -0.7778 is rounded off. Whereas -0.7778 is not rounded off in expected hence their comparison is false!

Is there any way to compare these two -0.7778888888.. variable with -0.7778?
Or some way to declare a variable as -0.77788888... ?

One thing I tried was -0.777888... = (7001/9000) and compare but seems such tricks don't work.

Strange! implementation is fine now, but tests aren't :(

Please let me know how to compare such two tensor elements.

[fmassa]

One way to do it is to use self.assertTrue((tensor1 - tensor2).abs().max() < tolerance). We use this a lot in the codebase, specially when dealing with floating point data. I would use something like that as well here

[oke-aditya]

Excellent advice. It's a new thing I learnt and will remember for next time too.
I guess now this PR is ready to merge. 👍

[fmassa]

Thanks a lot!