Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Dockerfile #1569

Merged
merged 4 commits into from
Apr 25, 2020
Merged

Create Dockerfile #1569

merged 4 commits into from
Apr 25, 2020

Conversation

justusschock
Copy link
Member

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 to update the docs?
  • Did you write any new necessary tests?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Adds a docker file to

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 Borda added the feature Is an improvement or enhancement label Apr 23, 2020
@@ -0,0 +1,40 @@
ARG CUDA_VERSION=10.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are older pytorch version compatible? I remember that a month ago even 1.4 supports cuda 10.1 and 9.2 only
There was also a comment that that from pt 1.5 they support cuda 10.2

So maybe add a test call if installed pytorch accesses cuda correctly so we don't produce useless images...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't matter, since pytorch also comes with precompiled cuda. I chose that, since this is the version that may be used, if somebody wants to compile custom stuff.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be important for Horovod and I believe Apex/AMP, as they both compile from CUDA source.

@mergify mergify bot requested a review from a team April 23, 2020 11:53
@justusschock
Copy link
Member Author

docker build currently fails, since horovod guys have a hard torch vision dependency, that installs unrestricted torch vision (defaults to latest if not specified by us): horovod/horovod#1898

@williamFalcon
Copy link
Contributor

@tgaddair

@tgaddair
Copy link
Contributor

tgaddair commented Apr 23, 2020

docker build currently fails, since horovod guys have a hard torch vision dependency, that installs unrestricted torch vision (defaults to latest if not specified by us): horovod/horovod#1898

Hey @justusschock @williamFalcon, I'll make a PR to remove the torchvision dep. But in the meantime, as I mentioned in the other thread, you should be able to change horovod[pytorch] to horovod in your requirements-extra.txt to unblock.

@Borda
Copy link
Member

Borda commented Apr 23, 2020

Hey @justusschock @williamFalcon, I'll make a PR to remove the torchvision dep. But in the meantime, as I mentioned in the other thread, you should be able to change horovod[pytorch] to horovod in your requirements-extra.txt to unblock.

we just did in #1573

@Borda
Copy link
Member

Borda commented Apr 24, 2020

failed to build:

Step 14/15 : RUN python -c "import pytorch_ligtning as pl; print(pl.__version__)"
 ---> Running in 5e7a518cec95
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'pytorch_ligtning'
The command '/bin/sh -c python -c "import pytorch_ligtning as pl; print(pl.__version__)"' returned a non-zero code: 1

@justusschock
Copy link
Member Author

Just a small typo. Fixed this :)

@mergify mergify bot requested a review from a team April 24, 2020 09:01
@Borda Borda added the ready PRs ready to be merged label Apr 24, 2020
@mergify mergify bot requested a review from a team April 24, 2020 09:12
@justusschock
Copy link
Member Author

@williamFalcon can we merge this?

@williamFalcon williamFalcon merged commit d1279af into master Apr 25, 2020
@Borda Borda deleted the dockerfile branch April 25, 2020 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants