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

Update cupy extras #11279

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Conversation

adrianeboyd
Copy link
Contributor

Description

  • Extend to v11
  • Add cupy-cuda11x and cupy-wheel
  • Update quickstart to use cupy-wheel for CUDA 10.2+

Types of change

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

* Extend to v11
* Add `cupy-cuda11x` and `cupy-wheel`
* Update quickstart to use `cupy-wheel` for CUDA 10.2+
@adrianeboyd adrianeboyd added the gpu Using spaCy on GPU label Aug 8, 2022
@adrianeboyd
Copy link
Contributor Author

I'm not sure about the extra name cuda-wheel or the way the quickstart looks.

If you use cupy-wheel for all the existing CUDA versions in the quickstart (leaving them in the list), then it defaults to the first one (10.2), which works because all the commands are the same, but then it looks a bit weird as a default. But I think saying "10.2+" is going to confuse people. ???

@adrianeboyd
Copy link
Contributor Author

I don't really like cuda-wheel as the name, but we can't use plain cuda either because that's already used for source cupy. Maybe we can switch to maintaining the extras:

# keep cuda as source extra
cuda =
    cupy>=5.0.0b4,<12.0.0
# cupy as source extra
cupy =
    cupy>=5.0.0b4,<12.0.0
# cupy-wheel as wheel extra
cupy-wheel =
    cupy-wheel>=11.0.0,<12.0.0

We can keep the existing cuda* ones as is, but then just maintain / recommend these in the future?

@svlandeg svlandeg requested a review from shadeMe August 17, 2022 14:06
Copy link
Contributor

@shadeMe shadeMe left a comment

Choose a reason for hiding this comment

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

Just one minor comment.

website/src/widgets/quickstart-install.js Outdated Show resolved Hide resolved
@adrianeboyd
Copy link
Contributor Author

adrianeboyd commented Sep 7, 2022

Looking at it again, I'm not sure cupy-wheel is stable enough that we want to recommend it yet across the board. It's not used in the main cupy install docs yet.

@polm
Copy link
Contributor

polm commented Sep 7, 2022

For the record, this is the thread for status of cupy-wheel, which does seem to still be experimental:

cupy/cupy#6688

It's not clearly stated, but it looks like it's supposed to work for CUDA 10.2 and not just 11+ - see the "supported packages" in the PR here.

It looks like there are some environments (like Docker) where there may be issues with it still, though it seems to be working fine for normal use.

@kmaehashi
Copy link

kmaehashi commented Sep 9, 2022

Hi, CuPy maintainer here.
Technical issues discussed in cupy/cupy#6688 has been resolved in v11 and I believe it's ready to use by downstream packages. Currently, it lacks documentation but we are going to do that in cupy/cupy#7005.

I'd suggest naming the extras_require as cuda-autodetect. The primary benefit of the cupy-wheel package is that it automatically finds the CUDA version installed in the users' environment so that they don't have to know about their CUDA version and select which cupy-cudaXXX to use. For users having multiple CUDA versions installed on the same machine, however, it could be ambiguous whether CUDA picked by cupy-wheel is the one user actually want to use (cupy-wheel always try to find the latest CUDA version installed), so I think it's better to keep providing cuda11{01234567x} extras as well to cover such case.

@adrianeboyd
Copy link
Contributor Author

Thanks for the info! cuda-autodetect sounds like a good name to use.

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@adrianeboyd adrianeboyd merged commit 6be6913 into explosion:master Sep 13, 2022
@svlandeg svlandeg added the install Installation issues label Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gpu Using spaCy on GPU install Installation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants