lilanxiao / Rotated_IoU

Differentiable IoU of rotated bounding boxes using Pytorch
MIT License
415 stars 64 forks source link

incorrect iou? #8

Closed filaPro closed 3 years ago

filaPro commented 4 years ago

Hi @lilanxiao ,

Maybe I'm missing something but:

device = torch.device('cuda:0')
box_0 = torch.tensor([.0, .0, 2., 2., .0], device=device)
box_1 = torch.tensor([.0, 2., 2., 2., .0], device=device)
print(cal_iou(box_0[None, None, ...], box_1[None, None, ...])[0].cpu().numpy())

returns [[0.33333334]]. However, these 2 boxes are not intersecting.

lilanxiao commented 4 years ago

@filaPro Hi, thank you for the issue and the example!

It's definitely a bug. It happens when two edges "perfectly" overlap. I've changed the box_1 in your code a little bit. With just a tiny offset, the function returns correct result. All following three values works.

EPSILON = 1e-6
device = torch.device('cuda:0')
box_0 = torch.tensor([.0, .0, 2., 2., .0], device=device)
box_1 = torch.tensor([EPSILON, 2, 2., 2., .0], device=device) # return 0
# box_1 = torch.tensor([.0, 2, 2-EPSILON, 2., .0], device=device) # return 0
# box_1 = torch.tensor([.0, 2, 2., 2., EPSILON], device=device) # return 5.960465e-08
print(cal_iou(box_0[None, None, ...], box_1[None, None, ...])[0].cpu().numpy())

It's probably not a big issue in practice because It's quite unlikely that the predicted bounding box and the ground truth would overlap in such a way.

But I'll still check my code again and let you know when I figure it out.

filaPro commented 4 years ago

Probably this EPSILON stuff is not the best idea here. After replacing

t = den_t / (num + EPSILON) 
mask_t = (t >= 0) * (t < 1)
...
u = - den_u / (num + EPSILON)
mask_u = (u >= 0) * (u < 1)  

with

t = den_t / num
t[num == .0] = -1.
mask_t = (t >= 0) * (t <= 1)
...
u = -den_u / num
u[num == .0] = -1.
mask_u = (u >= 0) * (u <= 1)  

everything looks fine.

filaPro commented 4 years ago

Looks like this case:

box_0 = torch.tensor([.0, .0, 2., 2., .0], device=device)
box_1 = torch.tensor([.0, 1., 2., 2., .0], device=device)

is also incorrect for now. However the solution above helps.

lilanxiao commented 4 years ago

@filaPro Nice catch! Thank you very much for the solution! It fixed the incorrect IoU value but I got nan when I ran the demo.py. Perhaps there is still some numerical instability somewhere.

lilanxiao commented 4 years ago

@filaPro I have fixed the IoU value bug and the stability issue in a new commit. Thank you again!

filaPro commented 4 years ago

Thanks a lot! Do you have an intuition why we need this t = den_t / (num + EPSILON)? I see that it is unstable otherwise. But nan values here was masked out by mask_t...

filaPro commented 4 years ago

For this case:

box_0 = torch.tensor([.0, .0, 2., 2., .0], device=device)
box_1 = torch.tensor([.0, 0., 2., 2., .0], device=device)

the answer is inf now. May be the problem is a little bit deeper.

lilanxiao commented 4 years ago

@filaPro For the nan problem. If I remember it correctly, nan follows 0 * nan = nan, according to the definition of IEEE.

I spent some time on this issue. The problem is how I formulate the problem. I split the problem to 4 sub-problems.

  1. find corners of one box located in anther box ("corners in box")
  2. find intersection points of edges ("intersection points")
  3. assume these corners in box and intersection points are the vertices of the intersection polygon, sort the vertices in clockwise or anti-clockwise order
  4. apply shoelace formula to get the area. Screenshot from 2020-10-26 16-25-57

The trouble is, how to define "corner in box" and "intersection points", when two edges are collinear. For example, with following two boxes, how many "corners in box" should be counted? 0 or 2 ? Where is the intersection points? Or do they exist?

box_0 = torch.tensor([.0, .0, 2., 2., .0], device=device)
box_1 = torch.tensor([.0, 2., 2., 2., .0], device=device)

My previous code didn't cover these edge cases well so it delivered wrong value. I tried to fix it and the result seems good now.

But to be honest, I'm not sure if it's the best solution and if all corner cases are covered... Also, I am not very clear why current version works...

filaPro commented 4 years ago

Looks like 1st question can be answered easily. However to answer properly on the 2nd question we need more code to manually intersect 2 edges, in case they are collinear. Current version still doesn't cover the case of 2 equal boxes :(

lilanxiao commented 4 years ago

I committed several times. I think the most recent commit (a63b0fbcffe9b144dd1e6b9fc9de8ffd96f35d32) covers the case of 2 equal boxes. Did you try it?

filaPro commented 4 years ago

Sorry, now it's fine. Everything works on some kind of magic :)

kevinghst commented 4 years ago

IOU still not working properly. params1 = params2 = [38, 120, 1.3, 20, 50]

I get .333, instead of 1...

lilanxiao commented 4 years ago

@kevinghst thank you for the issue! Yeah, it seems my dark magic didn't work. The true reason of the error was the vertices of the intersection polygon were duplicated (we only need 4, but would get 8) if the two boxes were exactly the same. In this case, the shoelace formula was broken. Also, there was still some numerically instability in my code.

I hope the bug is now properly fixed in commit 6267cefca26cbcd742bfb3cc4ae8064ba0b7a1da . This time I've modified the CUDA part and removed the magic. BTW, please remember to re-compile the CUDA extension.