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

[BC-breaking] Change default eps value of FrozenBN #2933

Merged
merged 6 commits into from
Oct 30, 2020

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Oct 29, 2020

Fixes #2599

The good news is that changing the FrozenBatchNorm2d default eps value from 0.0 to 1e-5 does not affect the accuracy of the detection models. You will see some tiny variations from the reported metrics but nothing concerning (could be the result of floating point errors). So I believe we can safely land the change without problems.

Click here for the accuracy stats of fasterrcnn_resnet50_fpn
IoU metric: bbox
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.369
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=100 ] = 0.585
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=100 ] = 0.398
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.210
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.403
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.480
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=  1 ] = 0.307
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 10 ] = 0.485
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.509
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.315
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.545
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.644
Click here for the accuracy stats of retinanet_resnet50_fpn
IoU metric: bbox
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.363
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=100 ] = 0.557
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=100 ] = 0.382
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.191
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.400
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.489
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=  1 ] = 0.314
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 10 ] = 0.500
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.539
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.339
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.580
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.695
Click here for the accuracy stats of maskrcnn_resnet50_fpn
IoU metric: bbox
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.379
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=100 ] = 0.591
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=100 ] = 0.410
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.215
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.414
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.494
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=  1 ] = 0.313
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 10 ] = 0.495
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.519
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.321
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.559
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.666
IoU metric: segm
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.346
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=100 ] = 0.560
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=100 ] = 0.368
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.156
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.373
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.507
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=  1 ] = 0.295
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 10 ] = 0.455
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.475
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.269
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.515
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.636
Click here for the accuracy stats of keypointrcnn_resnet50_fpn
IoU metric: bbox
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.546
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets=100 ] = 0.830
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets=100 ] = 0.597
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.380
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.628
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.702
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=  1 ] = 0.187
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 10 ] = 0.555
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets=100 ] = 0.636
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= small | maxDets=100 ] = 0.493
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets=100 ] = 0.700
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets=100 ] = 0.775
IoU metric: keypoints
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=   all | maxDets= 20 ] = 0.651
 Average Precision  (AP) @[ IoU=0.50      | area=   all | maxDets= 20 ] = 0.861
 Average Precision  (AP) @[ IoU=0.75      | area=   all | maxDets= 20 ] = 0.716
 Average Precision  (AP) @[ IoU=0.50:0.95 | area=medium | maxDets= 20 ] = 0.604
 Average Precision  (AP) @[ IoU=0.50:0.95 | area= large | maxDets= 20 ] = 0.730
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=   all | maxDets= 20 ] = 0.717
 Average Recall     (AR) @[ IoU=0.50      | area=   all | maxDets= 20 ] = 0.906
 Average Recall     (AR) @[ IoU=0.75      | area=   all | maxDets= 20 ] = 0.774
 Average Recall     (AR) @[ IoU=0.50:0.95 | area=medium | maxDets= 20 ] = 0.670
 Average Recall     (AR) @[ IoU=0.50:0.95 | area= large | maxDets= 20 ] = 0.784

The bad news is that the following detection unit-tests are brittle and changing the eps value causes them to break:

  • test_keypointrcnn_resnet50_fpn_cpu
  • test_keypointrcnn_resnet50_fpn_cuda
  • test_retinanet_resnet50_fpn_cpu

I tried updating the expecting values to new ones but unfortunately there is too much variation across different devices and platforms. As observed at #2812, the flakiness is caused by floating point errors and the use of unstable sort. The problem is the result of initializing the model weights with random values and using random input in the unit-tests.

To fix the problem I introduce a VERY hacky workaround where I switch the default value to 0.0 for specific unit-tests. I really don't like this hack but I don't see how else we can keep the unit-tests on and while changing the eps . If anyone has another idea, I'm happy to give it a try.

@datumbox datumbox requested a review from fmassa October 29, 2020 17:09
@datumbox datumbox changed the title [WIP] Change default eps value of FrozenBN Change default eps value of FrozenBN Oct 29, 2020
@codecov
Copy link

codecov bot commented Oct 29, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2933   +/-   ##
=======================================
  Coverage   73.41%   73.41%           
=======================================
  Files          99       99           
  Lines        8801     8801           
  Branches     1389     1389           
=======================================
  Hits         6461     6461           
  Misses       1915     1915           
  Partials      425      425           
Impacted Files Coverage Δ
torchvision/ops/misc.py 86.04% <ø> (ø)

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 1fe1e11...e4f3ce1. 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.

Thanks for the PR!

It's great that the changes on accuracy don't seem to be affected in a significant way.
I suspect the results before the change are also the same values, or does it decrease mAP by 0.1?

I have one remark about the hack, I think it can be made such that we don't need to change the implementation in the op itself

test/test_models.py Outdated Show resolved Hide resolved
@datumbox
Copy link
Contributor Author

datumbox commented Oct 30, 2020

Here are the baseline vs new comparisons:

fasterrcnn_resnet50_fpn:

  • MASTER: boxAP=0.370
  • BRANCH: boxAP=0.369

retinanet_resnet50_fpn:

  • MASTER: boxAP=0.364
  • BRANCH: boxAP=0.363

maskrcnn_resnet50_fpn:

  • MASTER: boxAP=0.379 / maskAP=0.346
  • BRANCH: boxAP=0.379 / maskAP=0.346

keypointrcnn_resnet50_fpn:

  • MASTER: boxAP=0.546 / kpAP=0.650
  • BRANCH: boxAP=0.546 / kpAP=0.651

So overall I think we are OK.

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!

@fmassa fmassa changed the title Change default eps value of FrozenBN [BC-breaking] Change default eps value of FrozenBN Oct 30, 2020
@fmassa fmassa merged commit 45e027c into pytorch:master Oct 30, 2020
@datumbox datumbox deleted the bugfix/frozenbn_eps branch October 30, 2020 12:02
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* Change default eps value of FrozenBN.

* Update the unit-tests.`

* Update the expected values.

* Revert the expected value and use original eps=0 value for flaky tests.

* Post init change of eps.

* Styles.
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
* Change default eps value of FrozenBN.

* Update the unit-tests.`

* Update the expected values.

* Revert the expected value and use original eps=0 value for flaky tests.

* Post init change of eps.

* Styles.
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.

Change default value of eps in FrozenBatchNorm to match BatchNorm
2 participants