[Feature] NMS update

[SemyonBevzuk]

Added "score_threshold" and "max_num" for the updated NMS.
This significantly improves performance in ONNX Runtime and reduces export times.
The table shows the average execution time of the NMS operation and the total inference time of the RetinaNet model:

RetinaNet (ONNX Runtime)	Befor (s)	Now (s)
NMS time	15.99	0.0002574
Total time	16.78	0.76766

This PR should be merged after merging this PR into mmcv.

[CLAassistant]

All committers have signed the CLA.

[shinya7y]

The order of score_factors seems inconsistent.
#4473

[SemyonBevzuk]

The order of score_factors seems inconsistent.
#4473

Yes, filtering by score_thr after multiplying by score_factor significantly degrades the quality metrics.
I have fixed this problem with APs drop in PyTorch. Here is the output of the tools/test.py:

python tools/test.py configs/yolo/yolov3_d53_mstrain-608_273e_coco.py  checkpoints/yolov3/yolov3_d53_mstrain-608_273e_coco-139f5633.pth --eval bbox

Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.335
Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=1000 ] = 0.563
Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=1000 ] = 0.352
Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=1000 ] = 0.196
Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=1000 ] = 0.364
Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=1000 ] = 0.436
Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.460
Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=300 ] = 0.460
Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=1000 ] = 0.460
Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=1000 ] = 0.297
Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=1000 ] = 0.483
Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=1000 ] = 0.586
OrderedDict([('bbox_mAP', 0.335), ('bbox_mAP_50', 0.563), ('bbox_mAP_75', 0.352), ('bbox_mAP_s', 0.196), ('bbox_mAP_m', 0.364), ('bbox_mAP_l', 0.436), ('bbox_mAP_copypaste', '0.335 0.563 0.352 0.196 0.364 0.436')])

[shinya7y]

Is the order issue unavoidable if torch.onnx.is_in_onnx_export()?

[ZwwWayne]

Hi @SemyonBevzuk ,
Thanks for your kind contribution. The unit tests fail, can you check that?

[SemyonBevzuk]

Hi @SemyonBevzuk ,
Thanks for your kind contribution. The unit tests fail, can you check that?

Hi @ZwwWayne,
Yes, unit tests fail, as this PR uses functionality from this PR in mmcv.
After adding new arguments (score_threshold, max_num) to the NMS operation, the unit tests should pass.

[RunningLeon]

@SemyonBevzuk Hi, you could install pre-commit in your environment and it would check the code style before each commit.

[RunningLeon]

@SemyonBevzuk There may be multiple places where batched_nms is called and max_num should be updated. For instance,
trident_roi_head:

mmdetection/mmdet/models/roi_heads/trident_roi_head.py

Line 36 in 931d96e

det_bboxes, keep = batched_nms(nms_bboxes, nms_scores, nms_inds,

[SemyonBevzuk]

@SemyonBevzuk There may be multiple places where batched_nms is called and max_num should be updated. For instance,
trident_roi_head:

mmdetection/mmdet/models/roi_heads/trident_roi_head.py

Line 36 in 931d96e

det_bboxes, keep = batched_nms(nms_bboxes, nms_scores, nms_inds,

Yes, I will look at other places, where batched_nms is called.

[AronLin]

With PR #4990 and open-mmlab/mmcv#957, I evaluated some released pytorch models.
The results shows that this PR does not adversely affect the original performance.

Model	Box AP	Seg AP	ori Box AP	ori Seg AP
mask_rcnn_r50_fpn_1x_coco	38.2	34.7	38.2	34.7
fcos_r50_caffe_fpn_gn-head_1x_coco	36.6	-	36.6	-
cascade_rcnn_r50_fpn_1x_coco	40.3	-	40.3	-
retinanet_r50_fpn_1x_coco	36.5	-	36.5	-
faster_rcnn_r50_fpn_1x_coco	37.4	-	37.4	-
yolov3_d53_mstrain-608_273e_coco	33.5	-	33.4	-

[AronLin]

ONNX Runtime performance with #4990 and open-mmlab/mmcv#957:

Model	Box AP	ori Box AP	pytorch
ssd300	25.6	25.6	25.6
retinanet_r50_fpn_1x_coco	36.4	36.4	36.5
yolov3_d53_mstrain-608_273e_coco	33.5	33.5	33.5

[RunningLeon]

@SemyonBevzuk Hi, since open-mmlab/mmcv#957 is merged. Could we fix conflicts and CI errors in this PR? Thanks a lot.

[codecov]

Codecov Report

Attention: Patch coverage is 46.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 65.05%. Comparing base (a4e04e8) to head (2d2223c).
Report is 605 commits behind head on master.

Files with missing lines	Patch %	Lines
mmdet/models/dense_heads/cascade_rpn_head.py	0.00%	3 Missing ⚠️
mmdet/models/dense_heads/fovea_head.py	0.00%	2 Missing ⚠️
mmdet/core/post_processing/bbox_nms.py	83.33%	0 Missing and 1 partial ⚠️
mmdet/models/dense_heads/rpn_head.py	66.66%	0 Missing and 1 partial ⚠️
mmdet/models/roi_heads/trident_roi_head.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4990      +/-   ##
==========================================
- Coverage   65.07%   65.05%   -0.02%     
==========================================
  Files         275      275              
  Lines       21238    21249      +11     
  Branches     3532     3536       +4     
==========================================
+ Hits        13820    13824       +4     
- Misses       6669     6673       +4     
- Partials      749      752       +3     

Flag	Coverage Δ
unittests	65.02% <46.66%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[RunningLeon]

@ZwwWayne @jshilong @hhaAndroid Hi, guys, do you have a conclusion and an agreed decision on this PR?