Fix `accumulate_grad_batches` on init #9652

rohitgr7 · 2021-09-22T21:01:00Z

What does this PR do?

Just a small fix to make sure that accumulate_grad_batches is an int all the time and we use gradient accumulation scheduler to define it's value.

self.trainer.accumulate_grad_batches = self.trainer.accumulation_scheduler.get_accumulate_grad_batches(0)

the reason to have this one even here though it's updated inside on_train_epoch_start is because I have seen users using it to calculate total training_steps which is usually used while defining some of the schedulers. So this is required at this point.

also in case a scheduler is passed inside callbacks then that is also taken care off now.

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

Was this discussed/approved via a GitHub issue? (not for typos and docs)
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?
Did you make sure to update the documentation with your changes? (if necessary)
Did you write any new necessary tests? (not for typos and docs)
Did you verify new and existing tests pass locally with your changes?
Did you list all the breaking changes introduced by this pull request?
Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

github-actions · 2021-09-22T21:38:12Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

codecov · 2021-09-22T21:57:32Z

Codecov Report

Merging #9652 (9e6cee9) into master (ce00053) will decrease coverage by 4%.
The diff coverage is 82%.

❗ Current head 9e6cee9 differs from pull request most recent head 829aa12. Consider uploading reports for the commit 829aa12 to get more accurate results

@@           Coverage Diff           @@
##           master   #9652    +/-   ##
=======================================
- Coverage      93%     89%    -4%     
=======================================
  Files         179     179            
  Lines       15286   15285     -1     
=======================================
- Hits        14177   13566   -611     
- Misses       1109    1719   +610

pytorch_lightning/trainer/connectors/training_trick_connector.py

github-actions · 2021-09-23T04:53:33Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

github-actions · 2021-09-23T04:53:56Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

tests/trainer/test_trainer.py

rohitgr7 · 2021-09-23T15:09:31Z

moved the logic to callback connector since GradientAccumulateCallback is also a callback and we need to take care for that if that is passed by user inside callback. if a callback is passed we check if the other argument is passed or not.. if both are present then a minconfig error is raised (we can check it to warning if required) else callback is considered as well.

pytorch_lightning/trainer/connectors/callback_connector.py

rohitgr7 · 2021-09-23T16:36:14Z

seems like there is some issue with ipus + accumulate_grad_batches.. need to explore more on this.

github-actions · 2021-09-23T18:24:53Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Co-authored-by: Carlos Mocholí <[email protected]>

pytorch_lightning/plugins/training_type/ipu.py

tests/accelerators/test_ipu.py

github-actions · 2021-09-24T15:36:28Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

github-actions · 2021-09-24T15:37:11Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

rohitgr7 · 2021-09-24T15:39:19Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

apologies for this.. initially I thought this is going to be a simple fix.. but turns out there were 2 more issues.

pytorch_lightning/trainer/trainer.py

pytorch_lightning/trainer/connectors/callback_connector.py

Borda · 2021-09-24T18:48:32Z

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

apologies for this.. initially I thought this is going to be a simple fix.. but turns out there were 2 more issues.

That is fine, take it easy...
@tchaton the bot is quite annoying 🐰

Co-authored-by: Rohit Gupta <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]>

rohitgr7 changed the title ~~Fix accumulate_grad_batches on init~~ Fix accumulate_grad_batches on init Sep 22, 2021

rohitgr7 marked this pull request as ready for review September 22, 2021 21:37

rohitgr7 requested review from awaelchli, Borda, carmocca, justusschock, kaushikb11, SeanNaren, tchaton and williamFalcon as code owners September 22, 2021 21:37

rohitgr7 self-assigned this Sep 22, 2021

rohitgr7 added the bug Something isn't working label Sep 22, 2021

rohitgr7 added this to the v1.4.x milestone Sep 22, 2021

awaelchli approved these changes Sep 23, 2021

View reviewed changes

pytorch_lightning/trainer/connectors/training_trick_connector.py Outdated Show resolved Hide resolved

carmocca approved these changes Sep 23, 2021

View reviewed changes

tests/trainer/test_trainer.py Outdated Show resolved Hide resolved

mergify bot added the ready PRs ready to be merged label Sep 23, 2021

rohitgr7 removed the ready PRs ready to be merged label Sep 23, 2021

mergify bot added the ready PRs ready to be merged label Sep 23, 2021

carmocca reviewed Sep 23, 2021

View reviewed changes

pytorch_lightning/trainer/connectors/callback_connector.py Outdated Show resolved Hide resolved

rohitgr7 requested review from ananthsub, awaelchli and carmocca September 23, 2021 16:26

rohitgr7 changed the title ~~Fix accumulate_grad_batches on init~~ [WIP] Fix accumulate_grad_batches on init Sep 23, 2021

rohitgr7 and others added 9 commits September 24, 2021 20:29

update error

83afee7

move to callback connector

07099a9

add test with callback

2799ac8

fix tests

1bcb528

Update pytorch_lightning/trainer/connectors/callback_connector.py

be7e524

Co-authored-by: Carlos Mocholí <[email protected]>

update ipu logic

1c9ce1c

rev

068d799

rev

56c3e5e

rev

a314f85

rohitgr7 force-pushed the bugfix/acc_grad branch from db0fbb1 to c63c00c Compare September 24, 2021 15:02

pls work

9e6cee9

rohitgr7 force-pushed the bugfix/acc_grad branch from 667b8bd to 9e6cee9 Compare September 24, 2021 15:18

rohitgr7 commented Sep 24, 2021

View reviewed changes

pytorch_lightning/plugins/training_type/ipu.py Show resolved Hide resolved

rohitgr7 commented Sep 24, 2021

View reviewed changes

tests/accelerators/test_ipu.py Show resolved Hide resolved

rohitgr7 changed the title ~~[WIP] Fix accumulate_grad_batches on init~~ Fix accumulate_grad_batches on init Sep 24, 2021

carmocca approved these changes Sep 24, 2021

View reviewed changes

awaelchli reviewed Sep 24, 2021

View reviewed changes

SeanNaren approved these changes Sep 24, 2021

View reviewed changes

awaelchli reviewed Sep 24, 2021

View reviewed changes

pytorch_lightning/trainer/connectors/callback_connector.py Outdated Show resolved Hide resolved

code review

829aa12

rohitgr7 enabled auto-merge (squash) September 24, 2021 18:23

rohitgr7 requested a review from awaelchli September 24, 2021 18:23

rohitgr7 merged commit 8fcdcb5 into master Sep 24, 2021

rohitgr7 deleted the bugfix/acc_grad branch September 24, 2021 18:51

awaelchli added a commit that referenced this pull request Sep 29, 2021

Fix accumulate_grad_batches on init (#9652)

f117e04

Co-authored-by: Rohit Gupta <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]>

awaelchli mentioned this pull request Sep 29, 2021

Weekly Patch Release v1.4.9 [full merge, no squash] #9758

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `accumulate_grad_batches` on init #9652

Fix `accumulate_grad_batches` on init #9652

rohitgr7 commented Sep 22, 2021 •

edited

Loading

github-actions bot commented Sep 22, 2021

codecov bot commented Sep 22, 2021 •

edited

Loading

github-actions bot commented Sep 23, 2021

github-actions bot commented Sep 23, 2021

rohitgr7 commented Sep 23, 2021

rohitgr7 commented Sep 23, 2021

github-actions bot commented Sep 23, 2021

github-actions bot commented Sep 24, 2021

github-actions bot commented Sep 24, 2021

rohitgr7 commented Sep 24, 2021

Borda commented Sep 24, 2021

Fix accumulate_grad_batches on init #9652

Fix accumulate_grad_batches on init #9652

Conversation

rohitgr7 commented Sep 22, 2021 • edited Loading

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

PR review

Did you have fun?

github-actions bot commented Sep 22, 2021

codecov bot commented Sep 22, 2021 • edited Loading

Codecov Report

github-actions bot commented Sep 23, 2021

github-actions bot commented Sep 23, 2021

rohitgr7 commented Sep 23, 2021

rohitgr7 commented Sep 23, 2021

github-actions bot commented Sep 23, 2021

github-actions bot commented Sep 24, 2021

github-actions bot commented Sep 24, 2021

rohitgr7 commented Sep 24, 2021

Borda commented Sep 24, 2021

Fix `accumulate_grad_batches` on init #9652

Fix `accumulate_grad_batches` on init #9652

rohitgr7 commented Sep 22, 2021 •

edited

Loading

codecov bot commented Sep 22, 2021 •

edited

Loading