[BUGFIX] AMP + Precision unscale grad #4441

tchaton · 2020-10-30T14:33:13Z

What does this PR do?

This PR does the follow:

upscale gradients only when optimizer_step is going to be called within AMPNativePlugin
call on_after_backward only if gradient accumulated
move the gradient tracking out to model.backward and only once gradient accumulated.

Fixes #4151
Fixes #4427

Before submitting

Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?
Did you verify new and existing tests pass locally with your changes?
If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

awaelchli · 2020-10-30T14:44:27Z

@tchaton not sure if this is WIP but I believe the test is missing a trainer.fit(model).

codecov · 2020-10-30T14:54:22Z

Codecov Report

Merging #4441 into master will decrease coverage by 0%.
The diff coverage is 75%.

@@          Coverage Diff           @@
##           master   #4441   +/-   ##
======================================
- Coverage      93%     93%   -0%     
======================================
  Files         113     113           
  Lines        8194    8197    +3     
======================================
+ Hits         7627    7629    +2     
- Misses        567     568    +1

tchaton · 2020-10-30T15:41:02Z

@tchaton not sure if this is WIP but I believe the test is missing a trainer.fit(model).

Thanks @awaelchli, wrong copy / paste from Notebook

pytorch_lightning/trainer/training_loop.py

ydcjeff · 2020-10-30T16:11:30Z

Just a notice, the tests are skipped on Drone

pytorch_lightning/plugins/native_amp.py

pep8speaks · 2020-10-30T17:36:57Z

Hello @tchaton! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-02 14:07:43 UTC

…rch-lightning into issue_4311

tchaton

Just a notice, the tests are skipped on Drone

Yes, drone is running 1.5 and AMP is introduced in 1.6.

pytorch_lightning/trainer/training_loop.py

ydcjeff · 2020-10-30T17:43:27Z

@tchaton okay, shall we try to upgrade to 1.6?

pytorch_lightning/trainer/training_loop.py

tests/plugins/test_amp_plugin.py

Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

ydcjeff · 2020-11-02T10:40:52Z

@tchaton changelog must be under [unreleased]

SeanNaren · 2020-11-02T16:37:01Z

Thanks @tchaton this was a good catch!

* move unscale within Native plugin * remove gradient tracking from lightning backward * forgot trainer.fit * typo * update * cleanup * set to 1.6 * typo * skip if below 1.6 strict * update changelog * remove useless code * Update tests/plugins/test_amp_plugin.py Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> * Update tests/plugins/test_amp_plugin.py Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> * update changelog * Update CHANGELOG.md Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> Co-authored-by: Jeff Yang <ydcjeff@outlook.com> (cherry picked from commit 102fa9e)

* move unscale within Native plugin * remove gradient tracking from lightning backward * forgot trainer.fit * typo * update * cleanup * set to 1.6 * typo * skip if below 1.6 strict * update changelog * remove useless code * Update tests/plugins/test_amp_plugin.py Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> * Update tests/plugins/test_amp_plugin.py Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> * update changelog * Update CHANGELOG.md Co-authored-by: Sean Naren <sean.narenthiran@gmail.com> Co-authored-by: Jeff Yang <ydcjeff@outlook.com>

move unscale within Native plugin

66f8dbd

tchaton requested review from ananyahjha93, awaelchli, Borda, justusschock, nateraw, SeanNaren, teddykoker and williamFalcon as code owners October 30, 2020 14:33

Merge branch 'master' into issue_4311

e644383

tchaton added 4 commits October 30, 2020 15:28

remove gradient tracking from lightning backward

20d2287

forgot trainer.fit

434f28c

typo

b23c071

update

fdb7d45

cleanup

81ad406

SeanNaren reviewed Oct 30, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Outdated Show resolved Hide resolved

awaelchli reviewed Oct 30, 2020

View reviewed changes

pytorch_lightning/plugins/native_amp.py Outdated Show resolved Hide resolved

set to 1.6

a8a8b13

tchaton added 3 commits October 30, 2020 17:37

Merge branch 'master' into issue_4311

2cd7eff

typo

bdee738

Merge branch 'issue_4311' of https://github.com/PyTorchLightning/pyto…

16fee66

…rch-lightning into issue_4311

tchaton commented Oct 30, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Show resolved Hide resolved

skip if below 1.6 strict

b74c63a

ydcjeff added the bug Something isn't working label Oct 30, 2020

ydcjeff added this to the 1.0.x milestone Oct 30, 2020

tchaton added 3 commits October 30, 2020 17:56

update changelog

9fc0d8a

remove useless code

b18d37f

Merge branch 'master' into issue_4311

1528c82

tchaton requested review from awaelchli, ydcjeff and SeanNaren November 2, 2020 08:57

justusschock approved these changes Nov 2, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Show resolved Hide resolved

Merge branch 'master' into issue_4311

d0c9136

SeanNaren suggested changes Nov 2, 2020

View reviewed changes

tests/plugins/test_amp_plugin.py Outdated Show resolved Hide resolved

tests/plugins/test_amp_plugin.py Outdated Show resolved Hide resolved

tchaton and others added 2 commits November 2, 2020 10:38

Update tests/plugins/test_amp_plugin.py

f3be241

Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

Update tests/plugins/test_amp_plugin.py

be6577d

Co-authored-by: Sean Naren <sean.narenthiran@gmail.com>

SeanNaren and others added 2 commits November 2, 2020 10:46

Merge branch 'master' into issue_4311

ace51c1

update changelog

99a827e

SeanNaren approved these changes Nov 2, 2020

View reviewed changes

Update CHANGELOG.md

a9cbb1d

tchaton added the ready PRs ready to be merged label Nov 2, 2020

Merge branch 'master' into issue_4311

2579090

SkafteNicki approved these changes Nov 2, 2020

View reviewed changes

Merge branch 'master' into issue_4311

5b8cdc3

SeanNaren merged commit 102fa9e into master Nov 2, 2020

SeanNaren deleted the issue_4311 branch November 2, 2020 16:36

SeanNaren mentioned this pull request Nov 3, 2020

Switch to PyTorch 1.6 in Drone CI #4393

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUGFIX] AMP + Precision unscale grad #4441

[BUGFIX] AMP + Precision unscale grad #4441

tchaton commented Oct 30, 2020 •

edited

Loading

awaelchli commented Oct 30, 2020

codecov bot commented Oct 30, 2020 •

edited

Loading

tchaton commented Oct 30, 2020

ydcjeff commented Oct 30, 2020

pep8speaks commented Oct 30, 2020 •

edited

Loading

tchaton left a comment

ydcjeff commented Oct 30, 2020

ydcjeff commented Nov 2, 2020

SeanNaren commented Nov 2, 2020

[BUGFIX] AMP + Precision unscale grad #4441

[BUGFIX] AMP + Precision unscale grad #4441

Conversation

tchaton commented Oct 30, 2020 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

awaelchli commented Oct 30, 2020

codecov bot commented Oct 30, 2020 • edited Loading

Codecov Report

tchaton commented Oct 30, 2020

ydcjeff commented Oct 30, 2020

pep8speaks commented Oct 30, 2020 • edited Loading

Comment last updated at 2020-11-02 14:07:43 UTC

tchaton left a comment

Choose a reason for hiding this comment

ydcjeff commented Oct 30, 2020

ydcjeff commented Nov 2, 2020

SeanNaren commented Nov 2, 2020

tchaton commented Oct 30, 2020 •

edited

Loading

codecov bot commented Oct 30, 2020 •

edited

Loading

pep8speaks commented Oct 30, 2020 •

edited

Loading