Skip to content

Modify expected value and threshold for retinanet unit-test. #2812

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 4 commits into from
Oct 16, 2020

Conversation

datumbox
Copy link
Contributor

Updating the score_thresh=0.013 for RetinaNet returns non-empty results. This fixes #2810.

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2812   +/-   ##
=======================================
  Coverage   73.26%   73.26%           
=======================================
  Files          99       99           
  Lines        8778     8778           
  Branches     1387     1387           
=======================================
  Hits         6431     6431           
  Misses       1920     1920           
  Partials      427      427           

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 072d8b2...a1b077e. Read the comment docs.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot !

Let's just wait until CircleCI gets back to normal before merging this PR.

@fmassa
Copy link
Member

fmassa commented Oct 14, 2020

Test failures seem related (and happen only on GPU). Maybe there are duplicate detections somewhere and due to floating point errors we get a slightly off prediction?

If debugging / fixing this is hard, we can skip the numerical tests for retinanet on the GPU (while performing the other tests), and re-enable those later?

@datumbox
Copy link
Contributor Author

@fmassa I agree. This is likely due to floating point errors, possibly made worse from the fact that all weights and input data are random. I'll have another look, hopefully we won't need to disable the GPU tests.

@datumbox datumbox marked this pull request as draft October 15, 2020 08:55
@datumbox
Copy link
Contributor Author

Unfortunately there is numerical and sort instability on this unit-test across platforms. Given that currently on master we don't sufficiently test retinanet, I think it's worth putting at least some tests in. As a workaround I disable some of the cuda checks but I plan to revisit this once I have a proper GPU setup.

@datumbox datumbox marked this pull request as ready for review October 16, 2020 09:56
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot @datumbox !

@fmassa fmassa merged commit 5e33cc8 into pytorch:master Oct 16, 2020
@datumbox datumbox deleted the bugfix/issue2810 branch October 16, 2020 09:59
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
…#2812)

* Modify expected value and threshold for retinanet unit-test.

* Disable tests on GPU

Co-authored-by: Francisco Massa <[email protected]>
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
…#2812)

* Modify expected value and threshold for retinanet unit-test.

* Disable tests on GPU

Co-authored-by: Francisco Massa <[email protected]>
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.

Empty expected values on test_retinanet_resnet50_fpn
2 participants