-
Notifications
You must be signed in to change notification settings - Fork 16
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
Let user decide which cupy version to use #67
Let user decide which cupy version to use #67
Conversation
Remove cupy from dependencies and add three extras where each requires a different cupy version: cupy (building from source), and the two prebuild cupy-cuda12x and cupy-cuda11x
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I felt like the general concensus on #66 was to remove the dependency altogether and add an import time check no?
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
@jacobtomlinson This would indeed remove the dependency and add an import time check. Additionally, this would add the possibility for the user to install cupy with a pip-extra. |
Sure, but I'm not sure the extras is a sustainable way to move forwards. I suspect there will be more turmoil around packaging as GPU packages continue to mature. I would recommend just linking to the cupy docs and leaving it at that. Otherwise things may get stale here quickly. |
Okay, I removed the possibility of installing cupy via extras. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @relativityhd! I might not get to a release this week, but will try to find time next month to do so.
Implement solution for #66 as discussed.
Remove cupy from dependencies and add three extras where each requires a different cupy version: cupy (building from source), and the two prebuild cupy-cuda12x and cupy-cuda11x
Closes #66