Fix max_batches with fast_dev_run. #2581

rohitgr7 · 2020-07-11T11:28:27Z

What does this PR do?

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

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 🙃

codecov · 2020-07-11T11:37:35Z

Codecov Report

Merging #2581 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #2581   +/-   ##
======================================
  Coverage      91%     91%           
======================================
  Files          70      70           
  Lines        5755    5755           
======================================
  Hits         5243    5243           
  Misses        512     512

Borda

LGTM

awaelchli

Thanks for the fix. Should we add a test? Pretty sure this will break in the future otherwise :))

pytorch_lightning/trainer/evaluation_loop.py

Borda · 2020-07-11T16:12:09Z

Thanks for the fix. Should we add a test? Pretty sure this will break in the future otherwise :))

Very much agree, pls add a test for it...

rohitgr7 · 2020-07-11T19:00:38Z

I initiated limit_{mode}_batches = 1 in trainer init itself because limit_{mode}_batches({mode}_percent_check) can be int now. Hope it doesn't break anything. Let me know if it's not correct I will revert it. Also added the test.

awaelchli

Great! Did you check that the test you added fails on master?

pytorch_lightning/trainer/training_loop.py

mergify · 2020-07-21T19:20:24Z

This pull request is now in conflict... :(

williamFalcon · 2020-07-22T13:50:20Z

@rohitgr7 let's merge this. but please check that the test you added does indeed fail on master

rohitgr7 · 2020-07-22T16:05:52Z

@williamFalcon Fails on master.

mergify · 2020-07-23T11:08:57Z

This pull request is now in conflict... :(

rohitgr7 · 2020-07-23T21:31:33Z

quick question: shouldn't this be (batch_idx + 1) % self.row_log_interval == 0?

awaelchli · 2020-07-24T04:18:31Z

@rohitgr7 good catch. I think you are right, a few lines below there is the equivalent for the save interval.
Perhaps fix/discuss that in an new PR?

pep8speaks · 2020-07-27T18:35:32Z

Hello @rohitgr7! Thanks for updating this PR.

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

Comment last updated at 2020-07-27 21:29:28 UTC

williamFalcon · 2020-07-27T18:39:00Z

added more tests to be extra sure :)

This reverts commit 4320329.

awaelchli · 2020-07-27T22:06:13Z

nice work! excited to see these bugfixes in master!

mergify bot requested a review from a team July 11, 2020 11:29

rohitgr7 changed the title ~~Fix fast dev run~~ Fix max_batches with fast_dev_run. Jul 11, 2020

Borda added the bug Something isn't working label Jul 11, 2020

Borda approved these changes Jul 11, 2020

View reviewed changes

mergify bot requested a review from a team July 11, 2020 13:38

awaelchli requested changes Jul 11, 2020

View reviewed changes

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

mergify bot requested a review from a team July 11, 2020 15:09

rohitgr7 commented Jul 11, 2020

View reviewed changes

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

rohitgr7 changed the title ~~Fix max_batches with fast_dev_run.~~ [WIP] Fix max_batches with fast_dev_run. Jul 11, 2020

awaelchli approved these changes Jul 11, 2020

View reviewed changes

pytorch_lightning/trainer/training_loop.py Show resolved Hide resolved

mergify bot requested a review from a team July 11, 2020 22:25

rohitgr7 changed the title ~~[WIP] Fix max_batches with fast_dev_run.~~ Fix max_batches with fast_dev_run. Jul 12, 2020

rohitgr7 requested review from Borda and removed request for a team July 13, 2020 18:15

mergify bot requested a review from a team July 13, 2020 18:15

Borda approved these changes Jul 15, 2020

View reviewed changes

Borda requested a review from justusschock July 15, 2020 09:47

Borda added the ready PRs ready to be merged label Jul 15, 2020

mergify bot requested a review from a team July 15, 2020 09:48

rohitgr7 mentioned this pull request Jul 21, 2020

Fix num_batches with fast_dev_run & multiple test dataloaders #2661

Closed

7 tasks

williamFalcon added the allowed_pre_1.0 label Jul 22, 2020

rohitgr7 and others added 8 commits July 27, 2020 13:48

fast_dev_run check

0d4a71b

changelog

ad7e370

explicit

15a4403

limit_batches with fast_dev_run in init

e87ee4e

add test

ff5bcc2

whitespace and comment fix

54bdc3e

comment and assertion

87c232f

added tests

ccc8708

rohitgr7 and others added 16 commits July 27, 2020 14:39

Fix fast_dev_run to run for all val_dataloaders

f83217b

fast_dev_run check

5c2b54e

changelog

691dd48

explicit

bea214f

limit_batches with fast_dev_run in init

44098de

add test

c7cb90e

whitespace and comment fix

92a2ada

comment and assertion

6554ac3

added tests

db0cad4

added tests

af472b5

added tests

6885d47

added tests

6a04853

added tests

5ae0ef5

update rtol

4320329

Revert "update rtol"

619c5ab

This reverts commit 4320329.

added tests

7f5c1d9

williamFalcon merged commit 84c507c into Lightning-AI:master Jul 27, 2020

rohitgr7 deleted the fix_fast_dev_run branch August 4, 2020 19:22

titu1994 mentioned this pull request Aug 20, 2020

Pin PTL, bump omegaconf NVIDIA/NeMo#1049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix max_batches with fast_dev_run. #2581

Fix max_batches with fast_dev_run. #2581

rohitgr7 commented Jul 11, 2020

codecov bot commented Jul 11, 2020

Borda left a comment

awaelchli left a comment

Borda commented Jul 11, 2020

rohitgr7 commented Jul 11, 2020

awaelchli left a comment

mergify bot commented Jul 21, 2020

williamFalcon commented Jul 22, 2020

rohitgr7 commented Jul 22, 2020

mergify bot commented Jul 23, 2020

rohitgr7 commented Jul 23, 2020

awaelchli commented Jul 24, 2020

pep8speaks commented Jul 27, 2020 •

edited

williamFalcon commented Jul 27, 2020

awaelchli commented Jul 27, 2020 •

edited

Fix max_batches with fast_dev_run. #2581

Fix max_batches with fast_dev_run. #2581

Conversation

rohitgr7 commented Jul 11, 2020

What does this PR do?

Before submitting

PR review

Did you have fun?

codecov bot commented Jul 11, 2020

Codecov Report

Borda left a comment

Choose a reason for hiding this comment

awaelchli left a comment

Choose a reason for hiding this comment

Borda commented Jul 11, 2020

rohitgr7 commented Jul 11, 2020

awaelchli left a comment

Choose a reason for hiding this comment

mergify bot commented Jul 21, 2020

williamFalcon commented Jul 22, 2020

rohitgr7 commented Jul 22, 2020

mergify bot commented Jul 23, 2020

rohitgr7 commented Jul 23, 2020

awaelchli commented Jul 24, 2020

pep8speaks commented Jul 27, 2020 • edited

Comment last updated at 2020-07-27 21:29:28 UTC

williamFalcon commented Jul 27, 2020

awaelchli commented Jul 27, 2020 • edited

pep8speaks commented Jul 27, 2020 •

edited

awaelchli commented Jul 27, 2020 •

edited