Issue with filter_boxes

[fmassa]

Hi,

Thanks a lot for the demo app, it looks very nice!

While playing with it, we noticed some unexpected behaviors of the model.

We initially were a bit confused by the presence of NMS in the model output, as DETR doesn't need NMS in general, so we looked a bit more into it.

It turns out that there is a couple of issues the way NMS is applied in filter_boxes.
Indeed, in there you applying NMS irrespective of the class label, and thus are removing important predictions.

In

dash-detr/model.py

Line 39 in 99474ba

keep = nms(boxes, scores, iou)

we could use the class labels to perform NMS on a per-class basis, using batched_nms. This is standard practice in object detectors that use NMS, such as Faster R-CNN.

Another thing is that nms expects a 1d tensor for the scores, but you are feeding a 2d tensor, so the values it is considering for NMS are most probably wrong. We should definitely add a check in torchvision so that this shows an error.

Here is the proposed change:

class_score, labels = scores.max(-1)
keep = torchvision.ops.boxes.batched_nms(boxes, class_score, labels, iou)

Comparison before and after

Results before (in the current state):

Results after the proposed fix:

---

Option to enable-disable NMS

As a separate comment, would it be possible to add a toggle flag to enable-disable NMS? DETR by default doesn't need NMS, so it would be great if this option could be added in the webapp.

Once again thanks for the great work!

[xhluca]

Happy you appreciate the app! Indeed, I noticed there was some overlap, so I thought about quickly adding non-max suppression, but I should definitely have spent some more time investigating the results. Overall, batch_nms looks a lot better, so I'm happy to merge and push it!

For the second point, I can add a RadioItems for toggling it on/off.

[fmassa]

Thanks a lot! Let us know when the patch is deployed.

[xhluca]

@fmassa The toggle was added both on master, and on the demo: https://dash-gallery.plotly.host/dash-detr