Skip to content
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

Remove empty boxes before NMS in detection models #1019

Merged
merged 1 commit into from
Jun 14, 2019

Conversation

fmassa
Copy link
Member

@fmassa fmassa commented Jun 14, 2019

Prior to this PR, we would pass (and return) empty boxes to the NMS.
This means that those boxes could be returned by the model (even though they might have very low scores and get filtered afterwards).
This is more of a safety check, and additionally avoid potential divisions by zero in the NMS.

@fmassa fmassa merged commit 5a7010f into pytorch:master Jun 14, 2019
@fmassa fmassa deleted the remove-empty-boxes-faster-rcnn branch June 14, 2019 12:32
@codecov-io
Copy link

codecov-io commented Jun 14, 2019

Codecov Report

Merging #1019 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1019      +/-   ##
==========================================
+ Coverage   63.27%   63.28%   +0.01%     
==========================================
  Files          65       65              
  Lines        5149     5151       +2     
  Branches      772      772              
==========================================
+ Hits         3258     3260       +2     
  Misses       1667     1667              
  Partials      224      224
Impacted Files Coverage Δ
torchvision/models/detection/roi_heads.py 56.89% <100%> (+0.24%) ⬆️

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 5ba9587...05cd3c0. Read the comment docs.

sprt added a commit to sprt/vision that referenced this pull request Dec 11, 2019
pytorch#1019 introduced a change that removes empty boxes in
postprocessing on the basis that, previously, empty boxes would actually
reach this postprocessing step, and that the model could therefore
output empty boxes (even though NMS would've most likely filtered them
out). It's essentially a safety check that'd be seldom needed.

However, that filtering causes dynamicity on the TPU (because the number
of empty boxes, if any, would be unknown). And in any case, we recently
introduced a change in PR pytorch#4 that purposefully pads the boxes tensor
with empty boxes, to avoid dynamicity. Therefore there's no point trying
to remove boxes that we use as padding, we'll just filter those out of
the output on the CPU.
sprt added a commit to sprt/vision that referenced this pull request Dec 11, 2019
pytorch#1019 introduced a change that removes empty boxes in
postprocessing on the basis that, previously, empty boxes would actually
reach this postprocessing step, and that the model could therefore
output empty boxes (even though NMS would've most likely filtered them
out). It's essentially a safety check that'd be seldom needed.

However, that filtering causes dynamicity on the TPU (because the number
of empty boxes, if any, would be unknown). And in any case, we recently
introduced a change in PR pytorch#4 that purposefully pads the boxes tensor
with empty boxes, to avoid dynamicity. Therefore there's no point trying
to remove boxes that we use as padding, we'll just filter those out of
the output on the CPU.
sprt added a commit to sprt/vision that referenced this pull request Dec 11, 2019
pytorch#1019 introduced a change that removes empty boxes in
postprocessing on the basis that, previously, empty boxes would actually
reach this postprocessing step, and that the model could therefore
output empty boxes (even though NMS would've most likely filtered them
out). It's essentially a safety check that'd be seldom needed.

However, that filtering causes dynamicity on the TPU (because the number
of empty boxes, if any, would be unknown). And in any case, we recently
introduced a change in PR pytorch#4 that purposefully pads the boxes tensor
with empty boxes, to avoid dynamicity. Therefore there's no point trying
to remove boxes that we use as padding, we'll just filter those out of
the output on the CPU.
sprt added a commit to jysohn23/vision that referenced this pull request Dec 14, 2019
pytorch#1019 introduced a change that removes empty boxes in
postprocessing on the basis that, previously, empty boxes would actually
reach this postprocessing step, and that the model could therefore
output empty boxes (even though NMS would've most likely filtered them
out). It's essentially a safety check that'd be seldom needed.

However, that filtering causes dynamicity on the TPU (because the number
of empty boxes, if any, would be unknown). And in any case, we recently
introduced a change in PR #4 that purposefully pads the boxes tensor
with empty boxes, to avoid dynamicity. Therefore there's no point trying
to remove boxes that we use as padding, we'll just filter those out of
the output on the CPU.
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