pytorch 1.6 #2745

Borda · 2020-07-28T20:31:13Z

What does this PR do?

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 🙃

Borda · 2020-07-28T21:17:55Z

@yukw777 there is some issue with loading, mind have a look

        if zipfile.is_zipfile(cached_file):
            with zipfile.ZipFile(cached_file) as cached_zipfile:
                members = cached_zipfile.infolist()
                if len(members) != 1:
>                   raise RuntimeError('Only one file(not dir) is allowed in the zipfile')
E                   RuntimeError: Only one file(not dir) is allowed in the zipfile

pep8speaks · 2020-07-29T18:04:56Z

Hello @Borda! Thanks for updating this PR.

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

Comment last updated at 2020-07-30 22:48:35 UTC

pytorch_lightning/trainer/distrib_data_parallel.py

pytorch_lightning/trainer/training_io.py

pytorch_lightning/trainer/distrib_data_parallel.py

pytorch_lightning/trainer/training_io.py

tests/models/test_tpu.py

codecov · 2020-07-29T18:39:14Z

Codecov Report

Merging #2745 into master will increase coverage by 0%.
The diff coverage is 67%.

@@          Coverage Diff           @@
##           master   #2745   +/-   ##
======================================
  Coverage      90%     90%           
======================================
  Files          76      76           
  Lines        6768    6776    +8     
======================================
+ Hits         6108    6117    +9     
+ Misses        660     659    -1

tests/callbacks/test_model_checkpoint.py

tests/models/test_tpu.py

yukw777 · 2020-07-29T20:55:22Z

pytorch 1.6 issue: pytorch/pytorch#42239

Borda · 2020-07-29T21:04:19Z

@yukw777 good job! it seems not that the update is not completely compatible with PT 1.3 mind check it?
we cal also just make a check of used PT version and adjust usage...

yukw777 · 2020-07-30T05:17:06Z

@tgaddair https://github.com/PyTorchLightning/pytorch-lightning/blob/8ab5bcda3d040fbe6fd616ff45793e3dfd1061ce/.github/workflows/ci-testing.yml#L104 why do we skip 3.8? can we add it back in?

Borda · 2020-07-30T07:02:11Z

why do we skip 3.8? can we add it back in?

if we can, let's re-enable it in another PR :]

tgaddair · 2020-07-30T12:53:49Z

@tgaddair

https://github.com/PyTorchLightning/pytorch-lightning/blob/8ab5bcda3d040fbe6fd616ff45793e3dfd1061ce/.github/workflows/ci-testing.yml#L104

why do we skip 3.8? can we add it back in?

Hey @yukw777, the underlying issue was fixed in Horovod 0.19.2. So provided the min version is at or above that, we can remove the checks against Python 3.8.

yukw777 · 2020-07-30T12:59:43Z

@tgaddair got it, thanks! sorry for the weird location of my comment. it was late at night and i mixed up the issues lol you can delete that and post it over here if you'd like.

tgaddair · 2020-07-30T13:04:04Z

No worries, comment has been moved.

Borda · 2020-07-30T13:14:36Z

@tgaddair @yukw777 resolving this issue here #2764

yukw777 · 2020-07-30T17:51:16Z

.github/workflows/ci-testing.yml

-      if: runner.os == 'windows' && matrix.requires == 'latest'
-      run: |
-        python -c "fname = 'requirements/base.txt' ; req = open(fname).read().replace('torch>=1.3', 'torch<1.5') ; open(fname, 'w').write(req)"
+    #- name: Setup Windows on Latest


@Borda do we just want to remove this instead of commenting it out?

probably at the time I commented I was not sure if it would work :]

hm? so we should remove it or keep it commented like that?

ananyahjha93 · 2020-07-30T22:30:26Z

tests/models/test_restore.py

@@ -309,7 +309,7 @@ def test_model_saving_loading(tmpdir):
    hparams_path = os.path.join(hparams_path, 'hparams.yaml')
    model_2 = EvalModelTemplate.load_from_checkpoint(
        checkpoint_path=new_weights_path,
-        hparams_file=hparams_path
+        hparams_file=hparams_path,


is this a good practice thing, adding a comma even after the last param?

I guess black does it too as with adding new parameter dif shows changed just one line instead of two if you have to add "," to the previous one...

Co-authored-by: Peter Yu <2057325+yukw777@users.noreply.github.com>

yukw777

Looks good overall! I'll let you decide what to do with the commented out block in ci-testing.yml :)

ethanwharris

👍

Borda added priority: 0 High priority task ci Continuous Integration labels Jul 28, 2020

mergify bot requested a review from a team July 28, 2020 20:32

Borda force-pushed the tests/pt-1.6 branch from 0a08f29 to 0e868df Compare July 28, 2020 20:46

Borda assigned yukw777 Jul 28, 2020

Borda added the Important label Jul 29, 2020

yukw777 reviewed Jul 29, 2020

View reviewed changes

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

Borda commented Jul 29, 2020

View reviewed changes

tests/callbacks/test_model_checkpoint.py Show resolved Hide resolved

Borda commented Jul 29, 2020

View reviewed changes

tests/models/test_tpu.py Show resolved Hide resolved

Borda marked this pull request as ready for review July 29, 2020 20:49

Borda force-pushed the tests/pt-1.6 branch from 6c90882 to c05a490 Compare July 30, 2020 07:04

Borda mentioned this pull request Jul 30, 2020

Horovod & py3.8 #2764

Merged

yukw777 reviewed Jul 30, 2020

View reviewed changes

mergify bot requested a review from a team July 30, 2020 17:51

Borda changed the title ~~pytorch 1.6~~ [blocked by #2764] pytorch 1.6 Jul 30, 2020

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

Borda requested review from yukw777 and ananyahjha93 July 30, 2020 21:38

ananyahjha93 reviewed Jul 30, 2020

View reviewed changes

ananyahjha93 approved these changes Jul 30, 2020

View reviewed changes

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

ananyahjha93 approved these changes Jul 30, 2020

View reviewed changes

mergify bot requested a review from a team July 30, 2020 22:32

Borda and others added 16 commits July 31, 2020 00:46

pt 1.6

76b5046

don't use the new zipfile serialization for now

f5eeb50

quick flake8 fixes

aee2778

remove unnecessary f

6953528

coalesce strings

fd8df3d

remove comma

e17b7ba

remove extra commas

e518aa8

Apply suggestions from code review

68acdb4

Co-authored-by: Peter Yu <2057325+yukw777@users.noreply.github.com>

set _use_new_zipfile_serialization to False only for pytorch 1.6.0

8b619cc

remove unnecessary comments

77de3d1

flake8 fixes

d24bd50

use pkg_resources instead of packaging

b6c8fd5

readme

730dc7b

format

60cc56b

version

61bea28

chlog

9af4dea

Borda force-pushed the tests/pt-1.6 branch from 71bca61 to 9af4dea Compare July 30, 2020 22:48

yukw777 approved these changes Jul 31, 2020

View reviewed changes

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

Borda requested review from ethanwharris, justusschock and rohitgr7 July 31, 2020 07:59

ethanwharris approved these changes Jul 31, 2020

View reviewed changes

Borda merged commit 06e8910 into master Jul 31, 2020

Borda deleted the tests/pt-1.6 branch July 31, 2020 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pytorch 1.6 #2745

pytorch 1.6 #2745

Borda commented Jul 28, 2020

Borda commented Jul 28, 2020

pep8speaks commented Jul 29, 2020 •

edited

codecov bot commented Jul 29, 2020 •

edited

yukw777 commented Jul 29, 2020

Borda commented Jul 29, 2020

yukw777 commented Jul 30, 2020

Borda commented Jul 30, 2020

tgaddair commented Jul 30, 2020 •

edited

yukw777 commented Jul 30, 2020

tgaddair commented Jul 30, 2020

Borda commented Jul 30, 2020

yukw777 Jul 30, 2020

Borda Jul 30, 2020

yukw777 Jul 31, 2020

ananyahjha93 Jul 30, 2020

Borda Jul 30, 2020

yukw777 left a comment

ethanwharris left a comment

pytorch 1.6 #2745

pytorch 1.6 #2745

Conversation

Borda commented Jul 28, 2020

What does this PR do?

PR review

Did you have fun?

Borda commented Jul 28, 2020

pep8speaks commented Jul 29, 2020 • edited

Comment last updated at 2020-07-30 22:48:35 UTC

codecov bot commented Jul 29, 2020 • edited

Codecov Report

yukw777 commented Jul 29, 2020

Borda commented Jul 29, 2020

yukw777 commented Jul 30, 2020

Borda commented Jul 30, 2020

tgaddair commented Jul 30, 2020 • edited

yukw777 commented Jul 30, 2020

tgaddair commented Jul 30, 2020

Borda commented Jul 30, 2020

yukw777 Jul 30, 2020

Choose a reason for hiding this comment

Borda Jul 30, 2020

Choose a reason for hiding this comment

yukw777 Jul 31, 2020

Choose a reason for hiding this comment

ananyahjha93 Jul 30, 2020

Choose a reason for hiding this comment

Borda Jul 30, 2020

Choose a reason for hiding this comment

yukw777 left a comment

Choose a reason for hiding this comment

ethanwharris left a comment

Choose a reason for hiding this comment

pep8speaks commented Jul 29, 2020 •

edited

codecov bot commented Jul 29, 2020 •

edited

tgaddair commented Jul 30, 2020 •

edited