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

Use latest supported CUDA version to install PyTorch #31

Merged
merged 6 commits into from
Jun 23, 2021
Merged

Use latest supported CUDA version to install PyTorch #31

merged 6 commits into from
Jun 23, 2021

Conversation

fepegar
Copy link
Contributor

@fepegar fepegar commented Jun 23, 2021

Before these changes, the installed PyTorch version was unnecessarily old for the installed drivers (430.50):

$ ltt install torch                                                                                   
Collecting torch==1.4.0+cu100
  Downloading https://download.pytorch.org/whl/cu100/torch-1.4.0%2Bcu100-cp36-cp36m-linux_x86_64.whl (723.9 MB)
     |██▏                             | 50.2 MB 43.4 MB/s eta 0:00:16^C

After these changes, the latest PyTorch compatible with my drivers is correctly used:

$ ltt install torch                                                                                   
Collecting torch==1.8.1+cu101
  Downloading https://download.pytorch.org/whl/cu101/torch-1.8.1%2Bcu101-cp36-cp36m-linux_x86_64.whl (763.6 MB)
     |██████▊                         | 160.8 MB 36.3 MB/s eta 0:00:17^C

Fixes #30.

Copy link
Owner

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Hey @fepegar, appreciate the PR! It only partially fixes #30, since now restricts the CUDA version to the most recent. In your use case you now will be able to use PyTorch compiled for CUDA 10.1, but no longer PyTorch compiled for CUDA 10.0 although that should be possible.

Anyway, if CI is happy I'm going to merge this, since it improves on the current implementation.

@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

Thanks for your feedback. I'll try to fix those tests.

@pmeier
Copy link
Owner

pmeier commented Jun 23, 2021

Let me know if you need help with that.

@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

Let me know if you need help with that.

Thanks! I had to modify the test, I hope they still make sense. I'm not very good at testing.

@pmeier
Copy link
Owner

pmeier commented Jun 23, 2021

I had to modify the test, I hope they still make sense. I'm not very good at testing.

No worries, this whole thing grabbed me. I'll make some time to fix this.

@pmeier
Copy link
Owner

pmeier commented Jun 23, 2021

Do you have the development environment set up? If yes, please run tox lint and commit the changes. If not, are you ok with me taking over the PR and fixing the linting issues?

@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

I thought running tox and pre-commit run --all-files would be enough to spot all the linting issues. I wasn't able to run tox lint but I checked the CI and I guess you mean tox -e lint. I'll fix these and commit in a second.

@pmeier
Copy link
Owner

pmeier commented Jun 23, 2021

Oh well, sorry for the bad directions. mypy is the problem here. Are you familiar with static type hinting?

@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

Are you familiar with static type hinting?

A little. Enough to solve this, hopefully! Let me know if there's anything else I should do.

Copy link
Owner

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot @fepegar!

@pmeier pmeier merged commit eff3ea9 into pmeier:master Jun 23, 2021
@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

Thanks for your guidance. I think I have a lot of software engineering to learn from this package, I'm looking forward to getting deeper into it and using the knowledge for TorchIO! I will recommend using ltt to install PyTorch from now on.

@pmeier
Copy link
Owner

pmeier commented Jun 23, 2021

If you or any user encounters some issues feel free to send them my way. Happy to make this better for everyone 🙂

@fepegar
Copy link
Contributor Author

fepegar commented Jun 23, 2021

Thanks, I will!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The found PyTorch version is too old
2 participants