Extend CI #44
Extend CI #44
Conversation
Codecov Report
@@ Coverage Diff @@
## master #44 +/- ##
======================================
Coverage ? 78%
======================================
Files ? 13
Lines ? 833
Branches ? 0
======================================
Hits ? 652
Misses ? 181
Partials ? 0 |
|
@williamFalcon it seems that your |
|
@williamFalcon pls create your free accounts and update the following badges: |
|
@Borda the code coverage badge was generated by doing this: |
Ah, was looking for something like codefactor. Good addition. Wondering why you suggest changing from circle ci to app veyor? what are the advantages? As mentioned above, i was thinking about auto codecov but i realized it was going to show something super low because a lot of code is GPU specific. Thus, I opted for running codecov and gou tests on a gpu machine. Sadly, not sure auto-codecov will work for this repo. Any suggestions on bridging that gap? (ie gpu use need) |
|
#44 (comment) ok, then the Coverage is fair, but it not very transparent from my point of view... as it is part of the repo and at the first glance it looks like just added illustration (especially when you see there 99%, it is like an ideal case...) no offence, just trying to help :) pip install codecov
codecov -t 17327163-8cca-4a5d-86c8-ca5f2ef700bcwhere the has is a unique token to your this/your project... |
|
#44 (comment) a good alternative to Codefactor is Codacy which has almost the same features... |
|
maybe Running GPU Executors - CircleCI but it seems to be running on AWS |
|
just added also CircleCI for python3.6 and python 3.7... |
|
it seems that also some tests are not very suitable for CI, they take too long... https://circleci.com/gh/Borda/pytorch-lightning/16 |
|
Great options. Let me address each individually. Re travis vs circle vs AppVeyorI mistyped haha I meant advantage over Travis. I agree there's no need for both (i've used both in the past, but i think i picked Travis for this because it could handle long tests and it was free). Re: Windows:Didn't officially try to support Windows as I think most people doing AI are using linux/macs (i know, i know, haha...), but in efforts to get this adopted by big older corps, I assume Windows support might be necessary. So, let's add the windows tests, and modify whatever we need to change in the library to achieve windows compatibility. (I really have no idea if it'll be that much different tbh). Hopefully we get Windows support for free. Re GPU testsThanks for reaching out to AWS. I spoke with the PyTorch team here at FB and the general consensus is that there really isn't a free way to run GPU tests. So, the suggestion was to allow people to run them on their own GPU machines (especially devs on the package). So, let's maybe table this for now until we find a good free solution? Azure would be great if they can support it (give the AI community some of that OpenAI money haha). Re test lengthA place to maybe pick up speed is to not download MNIST for every test (i realized this week that clearing the build folder also removes MNIST). That should provide a big speed-up. I think all the other tests are only training for 1 epoch and 1/10th of the data. There are 1 or 2 tests (CPU, GPU) respectively which train on more epochs to make sure it can achieve SOTA results on MNIST as a test. CodecovI agree it's not optimal and I also hate not having a thrid-party way to validate that the coverage wasn't faked. I didn't know about the submit option, so why don't we just do that? We can ask devs to run codecov and submit the results with a PR. I'd like to keep the coverage at 99%+ which means PRs have to be well covered. SummaryTo summarize, I think this is what we've converged on:
pip install codecov
codecov -t 17327163-8cca-4a5d-86c8-ca5f2ef700bc
Anything I'm missing? Things I owe:
Anything else? |
|
what about python3.5, your setup say |
Oh yeah... let's make the fix. Seems simple enough. I don't have strong opinions about which python version to support. At a minimum no support for 2. But if you have good reasons for starting at some version let's do that. But let's get rid of the formatting for that comment which seems trivial enough |
examples/new_project_templates/single_gpu_node_16bit_template.py
Outdated
Show resolved
Hide resolved
pytorch_lightning/models/trainer.py
Outdated
|
|
||
| # make DP and DDP mutually exclusive | ||
| # single GPU will also use DP with devices=[0] | ||
| have_gpus = self.data_parallel_device_ids is not None and len(self.data_parallel_device_ids) > 0 | ||
| if have_gpus: | ||
| if self.data_parallel_device_ids: |
There was a problem hiding this comment.
unfortunately the check needs to be:
self.data_parallel_device_ids is not None and len(self.data_parallel_device_ids) > 0
Case 1: is not None (user didn't pass gpus), so skip statement
Case 2: user did pass gpus AND it's more than a single GPU then we do whatever backend the user wants.
Case 3: use passed in a single GPU. In this case, we don't want to do DDP. We want to keep it as DP because DDP won't work well with a single GPU. So the above check leaves the default as 'dp' in this case which is what we want.
There was a problem hiding this comment.
I feel lost, what is the case when you want to enter the if block? not empty array?
Python 3.6.8 (default, Jan 14 2019, 11:02:34)
var = None
True if var else False
Out[3]: False
var = []
True if var else False
Out[5]: False
var = [2]
True if var else False
Out[7]: True
There was a problem hiding this comment.
1 GPUs. But if you don't do the None check before doing the check, it'll crash...
You want:
if more_than_1_gpus:
# enterBut can't do this:
more_than_1_gpus = len(self.data_parallel_device_ids) > 0because self.data_parallel_device_ids is None, so it'll crash.
Thus you have to check for that first as well (which is the case when Non GPU ids are passed)
| @@ -0,0 +1,47 @@ | |||
| # this file is *not* meant to cover or endorse the use of tox or pytest or testing in general, | |||
There was a problem hiding this comment.
what is this file for?
There was a problem hiding this comment.
it is a configuration of testing and defining test and formatting configurations at one place... see https://tox.readthedocs.io/en/latest
|
Good changes. Added comments inline |
|
|
|
there's no pytorch 1.1.0 support for windows... requires custom install |
|
I know about the missing pytorch for Win, just didn't have time yet to resolve it as you mention that Win is not the priority :) |
|
requested help for PyTorch installing... |
|
@Borda let's remove the build failing badge for windows until we have it resolved. i don't want to give the impression that the project is failing at the moment haha. |
|
@Borda can't access codecov getting a 504 error on their page... is this a stable service? (https://codecov.io/login/gh) |
|
they're back up |
|
it seems to be working for me, could you try to rproduce it? (I do not have sufficiently large GPU) |
|
@williamFalcon it seems that we (me) missed somewhere in the process the new badge for the license, could you please fix it... Thx |
* [PYT-210] Update Gallery cards * Tweak gallery sizing
Update script to load Search functionality
Extending actual CI and linked fixes/updates, reflecting #43