tests for legacy checkpoints by Borda · Pull Request #5223 · Lightning-AI/pytorch-lightning

Borda · 2020-12-21T17:18:09Z

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.
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; Bugfixes should be including in bug-fix release milestones (m.f.X) and features should be included in (m.X.b) releases.

Did you have fun?

Make sure you had fun coding 🙃

pep8speaks · 2020-12-21T17:18:13Z

Hello @Borda! Thanks for updating this PR.

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

Comment last updated at 2021-01-08 13:53:05 UTC

codecov · 2020-12-21T17:38:46Z

Codecov Report

Merging #5223 (138bcc3) into master (4c6f36e) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #5223   +/-   ##
======================================
  Coverage      93%     93%           
======================================
  Files         134     134           
  Lines        9994    9994           
======================================
  Hits         9309    9309           
  Misses        685     685

Borda · 2020-12-22T17:35:35Z

OK still the need to pull the data for (or skip these envs):

Drone with GPUs
TPU test docker image

.github/workflows/ci_test-conda.yml

.github/workflows/release-pypi.yml

tests/checkpointing/test_legacy_checkpoints.py

Borda · 2020-12-23T16:19:16Z

@tchaton mind review as code-owner?

Borda · 2020-12-29T18:02:54Z

@awaelchli @tchaton pls review ^^

awaelchli

looking very good! so the python script gets copied to the same location we save the checkpoint, to keep full reproducibility, right? should we maybe set a seed too? just to be sure?

Borda · 2020-12-29T18:19:56Z

looking very good! so the python script gets copied to the same location we save the checkpoint, to keep full reproducibility, right? should we maybe set a seed too? just to be sure?

the setting seed is a good point even not sure if it is needed as we do not check any metrics nor weights...
we can add it follow-up PR as we need to add more checkpoints anyway, this is mainly PoC that the flow works fine :]

tchaton · 2020-12-29T19:53:45Z

tests/checkpointing/test_legacy_checkpoints.py

+
+# todo: add more legacy checkpoints :]
+@pytest.mark.parametrize("pl_version", [
+    "0.10.0", "1.0.0", "1.0.1", "1.0.2", "1.0.3", "1.0.4", "1.0.5", "1.0.6", "1.0.7", "1.0.8"


Any chance we could add 1.1.x too ?

yes, as mentioned #5223 (comment) this is PoC and the next PR will add 0.{6,7,8,9,10}.x and 1.1.x and eventually seeds...

tchaton · 2020-12-29T19:58:25Z

looking very good! so the python script gets copied to the same location we save the checkpoint, to keep full reproducibility, right? should we maybe set a seed too? just to be sure?

@Borda It seems to be copied but not used right ?

Borda · 2020-12-29T20:05:46Z

looking very good! so the python script gets copied to the same location we save the checkpoint, to keep full reproducibility, right? should we maybe set a seed too? just to be sure?

@Borda It seems to be copied but not used right ?

yes, next PR #5223 (comment)

Borda · 2021-01-11T08:25:09Z

@carmocca @akihironitta wants to take it over and some more versions? :]

* wip * generate * clean * tests * copy * download * download * download * download * download * download * download * download * download * download * download * flake8 * extend * aws * extension * pull * pull * pull * pull * pull * pull * pull * try * try * try * got it * Apply suggestions from code review (cherry picked from commit 72525f0)

Borda added ci Continuous Integration checkpointing Related to checkpointing labels Dec 21, 2020

Borda added this to the 1.1.x milestone Dec 21, 2020

Borda marked this pull request as ready for review December 22, 2020 17:26

Borda requested review from SeanNaren, awaelchli, justusschock, tchaton and williamFalcon as code owners December 22, 2020 17:26

tchaton reviewed Dec 22, 2020

View reviewed changes

.github/workflows/ci_test-conda.yml Outdated Show resolved Hide resolved

.github/workflows/release-pypi.yml Outdated Show resolved Hide resolved

tests/checkpointing/test_legacy_checkpoints.py Show resolved Hide resolved

Borda requested a review from tchaton December 22, 2020 22:24

Borda commented Dec 23, 2020

View reviewed changes

tests/checkpointing/test_legacy_checkpoints.py Show resolved Hide resolved

SeanNaren approved these changes Dec 23, 2020

View reviewed changes

Borda added the ready to be merged PRs ready to be merged label Dec 23, 2020

Borda enabled auto-merge (squash) December 23, 2020 13:07

Borda added the priority: 1 Medium priority task label Dec 23, 2020

Borda self-assigned this Dec 29, 2020

awaelchli approved these changes Dec 29, 2020

View reviewed changes

tchaton approved these changes Dec 29, 2020

View reviewed changes

Borda added 3 commits January 7, 2021 12:04

wip

d0f41f7

generate

273c067

clean

523a785

Borda and others added 11 commits January 7, 2021 12:04

try

51ae29c

try

d698b34

got it

57b5fea

Apply suggestions from code review

5073287

curl

6ef2120

downloader-cli

b25710d

dw

1519dcd

dw

402abda

dw

a7eecc7

dw

0a1176e

dw

9ebcbed

Borda force-pushed the legacy-checkpoints branch from 1dd5e3e to bba9afe Compare January 7, 2021 11:05

ignore

138bcc3

Borda force-pushed the legacy-checkpoints branch from bba9afe to 138bcc3 Compare January 8, 2021 13:53

williamFalcon approved these changes Jan 8, 2021

View reviewed changes

justusschock approved these changes Jan 8, 2021

View reviewed changes

Borda merged commit 72525f0 into master Jan 8, 2021

Borda deleted the legacy-checkpoints branch January 8, 2021 15:36

Borda mentioned this pull request Jan 11, 2021

populate some more legacy checkpoints #5457

Merged

12 tasks

Conversation

Borda commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Before submitting

PR review

Did you have fun?

Uh oh!

pep8speaks commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-01-08 13:53:05 UTC

Uh oh!

codecov bot commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Borda commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Borda commented Dec 23, 2020

Uh oh!

Borda commented Dec 29, 2020

Uh oh!

awaelchli left a comment

Choose a reason for hiding this comment

Uh oh!

Borda commented Dec 29, 2020

Uh oh!

tchaton Dec 29, 2020

Choose a reason for hiding this comment

Uh oh!

Borda Dec 29, 2020

Choose a reason for hiding this comment

Uh oh!

tchaton commented Dec 29, 2020

Uh oh!

Borda commented Dec 29, 2020

Uh oh!

Borda commented Jan 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Borda commented Dec 21, 2020 •

edited

Loading

pep8speaks commented Dec 21, 2020 •

edited

Loading

codecov bot commented Dec 21, 2020 •

edited

Loading

Borda commented Dec 22, 2020 •

edited

Loading