Skip to content

Fix inconsistent NMS implementation between CPU and CUDA #1556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 5, 2019

Conversation

fmassa
Copy link
Member

@fmassa fmassa commented Nov 5, 2019

There was an inconsistency in how nms_threshold was handled in NMS between CPU and GPU. More precisely, the difference arises with proposals that satisfy overlap == nms_threshold.

We used ovr >= iou_threshold for CPU

if (ovr >= iou_threshold)
and ovr > iou_threshold for CUDA
if (devIoU<T>(cur_box, block_boxes + i * 4) > iou_threshold) {

This was caught during test refactorings in #1551, which exposed this inconsistency.

cc @rbgirshick @ppwwyyxx

@codecov-io
Copy link

codecov-io commented Nov 5, 2019

Codecov Report

Merging #1556 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1556   +/-   ##
=======================================
  Coverage   65.48%   65.48%           
=======================================
  Files          90       90           
  Lines        7080     7080           
  Branches     1077     1077           
=======================================
  Hits         4636     4636           
  Misses       2135     2135           
  Partials      309      309

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 8909ff4...335aa76. Read the comment docs.

@fmassa fmassa merged commit 4897402 into pytorch:master Nov 5, 2019
@fmassa fmassa deleted the fix-inconsistent-nms-cpu branch November 5, 2019 20:20
@fmassa fmassa mentioned this pull request Mar 31, 2020
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.

2 participants