repair CI for Win #2358

Borda · 2020-06-25T07:51:08Z

What does this PR do?

it seems like Win CI was falling but the checks were passed
https://github.com/PyTorchLightning/pytorch-lightning/pull/2356/checks?check_run_id=805809911

The issue was reported in nedbat/coveragepy#1003
Fixes #2347
Fixes #2357

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 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-06-25T08:18:39Z

Codecov Report

Merging #2358 into master will decrease coverage by 0%.
The diff coverage is 20%.

@@          Coverage Diff           @@
##           master   #2358   +/-   ##
======================================
- Coverage      88%     88%   -0%     
======================================
  Files          70      70           
  Lines        5555    5567   +12     
======================================
+ Hits         4889    4893    +4     
- Misses        666     674    +8

Borda · 2020-06-25T08:48:07Z

@awaelchli, failing for Windows:

C:\hostedtoolcache\windows\Python\3.6.8\x64\lib\site-packages\py\_path\local.py:704: in pyimport
    __import__(modname)
pytorch_lightning\metrics\__init__.py:1: in <module>
    from pytorch_lightning.metrics.converters import numpy_metric, tensor_metric
pytorch_lightning\metrics\converters.py:220: in <module>
    reduce_op: Optional[torch.distributed.ReduceOp] = None,
E   AttributeError: module 'torch.distributed' has no attribute 'ReduceOp'

pep8speaks · 2020-06-25T11:40:39Z

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-06-26 22:50:53 UTC

justusschock · 2020-06-25T11:53:16Z

@Borda the distributed part is fixed. only the horovod part still persists

Borda · 2020-06-25T13:43:21Z

@tgaddair we may have some problem with Horovod on Win, is it supported?
Otherwise, we may skip it for now :] @justusschock ^^ @awaelchli

justusschock

In the horovod test, don't we also have tp either skip it or change the backend and remove the horovod size asserts?

tgaddair · 2020-06-25T14:41:47Z

Hey @Borda, Horovod is not supported on Windows. Thanks for checking!

pytorch_lightning/metrics/converters.py

Borda · 2020-06-25T18:25:47Z

@tgaddair this is very strange as it is in try/except

tests\models\data\horovod\train_default_model.py:25: in <module>
    import horovod.torch as hvd
E   ModuleNotFoundError: No module named 'horovod

Borda · 2020-06-25T21:46:27Z

@awaelchli @justusschock @PyTorchLightning/core-contributors any idea why it fails even during the test collection before running any test? just for Windows... :/

Borda · 2020-06-25T23:18:59Z

fixed the failing but for some reason, the Win CI is super slow...

awaelchli · 2020-06-26T04:27:02Z

The tests became slow for me since the ddp mode was switched from spawn. I'm pretty sure that's it.

Borda · 2020-06-26T08:03:58Z

The tests became slow for me since the ddp mode was switched from spawn. I'm pretty sure that's it.

so shall we test by default ddp_spawn?

williamFalcon · 2020-06-26T12:45:39Z

sure that would work. but we need at least one ddp test.

awaelchli · 2020-06-26T12:47:05Z

try num_workers=0 in tests. it will speed up on windows. but probably on linux and mac it will be slower then.
this is an empirical finding, i can not explain why num_workers=0 on windows is faster :)

Borda · 2020-06-26T12:52:30Z

sure that would work. but we need at least one ddp test.

it runs all tests for all ddp as before just replace the default one, so all functionality is tested

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

…on Windows (#2294) * Fix load_from_checkpoint() not working with URL on Windows * Update CHANGELOG * Update CHANGELOG.md Co-authored-by: Peter Yu <2057325+yukw777@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Peter Yu <2057325+yukw777@users.noreply.github.com>

…torch-lightning into tests/ci-win

awaelchli

this was a tough one :)

yukw777 · 2020-06-27T10:50:15Z

@awaelchli, sorry for the late response! I’ve been traveling for the past few days and just saw this. It seems like you guys figured it out though. What was the issue with Windows?

yukw777 · 2020-06-27T10:53:23Z

tests/conftest.py

@@ -48,7 +48,7 @@ def translate_path(self, path):
        class ThreadingHTTPServer(ThreadingMixIn, HTTPServer):
            daemon_threads = True

-    with ThreadingHTTPServer(('', 0), Handler) as server:
+    with ThreadingHTTPServer(('localhost', 0), Handler) as server:


hey no problem.
yep that was it, Windows didn't like the 0.0.0.0 address :)

Borda · 2020-06-27T11:15:14Z

There were a couple of them as all Win tests were ignored for a few weeks or months...

Borda added the ci Continuous Integration label Jun 25, 2020

mergify bot requested a review from a team June 25, 2020 07:51

Borda mentioned this pull request Jun 25, 2020

wrong exit status for Windows CI nedbat/coveragepy#1003

Closed

Borda added the bug Something isn't working label Jun 25, 2020

awaelchli linked an issue Jun 25, 2020 that may be closed by this pull request

Error when importing metrics on Windows without DDP support #2347

Closed

Borda marked this pull request as ready for review June 25, 2020 14:09

Borda added the priority: 0 High priority task label Jun 25, 2020

Borda requested review from awaelchli, justusschock, SkafteNicki and williamFalcon June 25, 2020 14:09

justusschock requested changes Jun 25, 2020

View reviewed changes

mergify bot requested a review from a team June 25, 2020 14:24

Borda commented Jun 25, 2020

View reviewed changes

pytorch_lightning/metrics/converters.py Show resolved Hide resolved

Borda requested a review from justusschock June 25, 2020 22:05

Borda force-pushed the tests/ci-win branch from 86cdf0f to f2a47ca Compare June 26, 2020 11:15

Borda and others added 10 commits June 26, 2020 21:47

os

221fbd5

npm

b4e5622

Fix load_from_checkpoint() not working with URL on Windows

ae869f9

Update CHANGELOG

40d9f62

Update CHANGELOG.md

8130dfa

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

Apply suggestions from code review

ddd97f5

fix

41b51e7

fix meta tags creating empty lines

54f04fa

pyright

f0584a0

node

3f32abc

Borda force-pushed the tests/ci-win branch from 326d0e0 to 3f32abc Compare June 26, 2020 19:48

Borda added this to the 0.9.0 milestone Jun 26, 2020

fix httpserver address

431f7d4

awaelchli changed the title ~~repaire CI for Win~~ repair CI for Win Jun 26, 2020

drop tutils.default_trainer_options

eb10236

awaelchli mentioned this pull request Jun 26, 2020

Better fix for load_from_checkpoint() not working with absolute path on Windows #2294

Merged

7 tasks

Borda and others added 4 commits June 27, 2020 00:29

imports

a7b7151

drop duplicate

06c0b6d

Merge branch 'tests/ci-win' of https://github.com/PyTorchLightning/py…

e7132be

…torch-lightning into tests/ci-win

awaelchli approved these changes Jun 26, 2020

View reviewed changes

mergify bot requested a review from a team June 26, 2020 22:51

Borda added the ready PRs ready to be merged label Jun 26, 2020

williamFalcon merged commit f1c9693 into master Jun 27, 2020

awaelchli deleted the tests/ci-win branch June 27, 2020 09:46

yukw777 reviewed Jun 27, 2020

View reviewed changes

Borda mentioned this pull request Jun 29, 2020

testing gets stuck when num_workers is set to value >0 in tests/base/model_utilities.py #2421

Closed

Zymrael mentioned this pull request Jul 2, 2020

Known issues and incompatibilities DiffEqML/torchdyn#9

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

repair CI for Win #2358

repair CI for Win #2358

Borda commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

Borda commented Jun 25, 2020 •

edited

Loading

pep8speaks commented Jun 25, 2020 •

edited

Loading

justusschock commented Jun 25, 2020

Borda commented Jun 25, 2020 •

edited

Loading

justusschock left a comment

tgaddair commented Jun 25, 2020

Borda commented Jun 25, 2020

Borda commented Jun 25, 2020

Borda commented Jun 25, 2020

awaelchli commented Jun 26, 2020

Borda commented Jun 26, 2020

williamFalcon commented Jun 26, 2020

awaelchli commented Jun 26, 2020 •

edited

Loading

Borda commented Jun 26, 2020

awaelchli left a comment

yukw777 commented Jun 27, 2020

yukw777 Jun 27, 2020

awaelchli Jun 27, 2020

Borda commented Jun 27, 2020

repair CI for Win #2358

repair CI for Win #2358

Conversation

Borda commented Jun 25, 2020 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

codecov bot commented Jun 25, 2020 • edited Loading

Codecov Report

Borda commented Jun 25, 2020 • edited Loading

pep8speaks commented Jun 25, 2020 • edited Loading

Comment last updated at 2020-06-26 22:50:53 UTC

justusschock commented Jun 25, 2020

Borda commented Jun 25, 2020 • edited Loading

justusschock left a comment

Choose a reason for hiding this comment

tgaddair commented Jun 25, 2020

Borda commented Jun 25, 2020

Borda commented Jun 25, 2020

Borda commented Jun 25, 2020

awaelchli commented Jun 26, 2020

Borda commented Jun 26, 2020

williamFalcon commented Jun 26, 2020

awaelchli commented Jun 26, 2020 • edited Loading

Borda commented Jun 26, 2020

awaelchli left a comment

Choose a reason for hiding this comment

yukw777 commented Jun 27, 2020

yukw777 Jun 27, 2020

Choose a reason for hiding this comment

awaelchli Jun 27, 2020

Choose a reason for hiding this comment

Borda commented Jun 27, 2020

Borda commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

Borda commented Jun 25, 2020 •

edited

Loading

pep8speaks commented Jun 25, 2020 •

edited

Loading

Borda commented Jun 25, 2020 •

edited

Loading

awaelchli commented Jun 26, 2020 •

edited

Loading