Skip to content

Conversation

@ilijad
Copy link

@ilijad ilijad commented Oct 10, 2024

In the bbox_nms the box classes valid_cls were provided to batched_nms instead of box scores valid_con


batch_idx, *_ = torch.where(valid_mask)
nms_idx = batched_nms(valid_box, valid_cls, batch_idx, nms_cfg.min_iou)
nms_idx = batched_nms(valid_box, valid_con, batch_idx, nms_cfg.min_iou)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you put named arguments for the sake of readability?

Suggested change
nms_idx = batched_nms(valid_box, valid_con, batch_idx, nms_cfg.min_iou)
nms_idx = batched_nms(boxes=valid_box, scores=valid_con, idxs=batch_idx, iou_threshold=nms_cfg.min_iou)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@ilijad
Copy link
Author

ilijad commented Oct 10, 2024

Problem example

import torch
from torchvision.ops import batched_nms # type: ignore

valid_box = torch.Tensor([[10,10,20,20],[12,12,22,21]])
valid_con =torch.Tensor([0.3, 0.9])
batch_idx = torch.Tensor([0,0])
valid_cls = torch.Tensor([1,1])

rez_con = batched_nms(valid_box,valid_con,batch_idx, 0.5)
rez_cls = batched_nms(valid_box,valid_cls,batch_idx, 0.5)

print(rez_con, rez_cls) # rez_con = 1 rez_cls = 0
valid_con_reversed = torch.Tensor([0.9,0.3])

rez_con = batched_nms(valid_box,valid_con_reversed,batch_idx, 0.5)
rez_cls = batched_nms(valid_box,valid_cls,batch_idx, 0.5)
print(rez_con, rez_cls) # rez_con = 0 rez_cls = 0

different results obtained because of valid_box box order

@henrytsui000
Copy link
Member

Hi,

I believe this issue has already been fixed in PR #76. However, the DEPLOY branch hasn't been merged into the main branch yet. It will be merged later.

Best regards,
Henry Tsui

@ilijad
Copy link
Author

ilijad commented Oct 11, 2024

Yes you are right. I'll close the pull request.

@ilijad ilijad closed this Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants