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

Avoid DeprecationWarning in cupy dispatch registration #7836

Merged
merged 1 commit into from May 15, 2023

Conversation

wence-
Copy link
Contributor

@wence- wence- commented May 15, 2023

In cupy 12 and above, cusparse should be imported from cupyx, not cupy itself.

  • Tests added / passed
  • Passes pre-commit run --all-files

In cupy 12 and above, cusparse should be imported from cupyx, not cupy
itself.
Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

I can confirm this resolves the issue described in rapidsai/dask-cuda#1174. Thanks @wence- !

@github-actions
Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       26 files  ±0         26 suites  ±0   15h 51m 9s ⏱️ + 2m 39s
  3 600 tests ±0    3 493 ✔️ +1     105 💤 ±0  2  - 1 
45 553 runs  ±0  43 389 ✔️ +3  2 162 💤  - 2  2  - 1 

For more details on these failures, see this check.

Results for commit ac0dea2. ± Comparison against base commit 693ad9d.

@quasiben
Copy link
Member

Thanks all. Merging in

@quasiben quasiben merged commit 6303f95 into dask:main May 15, 2023
28 of 33 checks passed
@wence- wence- deleted the wence/fix/cupy-12-import branch May 15, 2023 15:34
@jakirkham
Copy link
Member

jakirkham commented May 15, 2023

This MatDescriptor serialization code was a workaround for a CuPy pre-8.0.0 issue ( cupy/cupy#3061 ). It was fixed in PR ( cupy/cupy#3157 ).

Maybe for newer CuPy's we skip this workaround. Alternatively we could just drop this workaround and request users have at least CuPy >=8.0.0 installed.

Edit: Maybe the dask & cuda serializers are still of some value. Though it might be worth checking if dask & cuda serialization would be preserved for CuPy sparse matrices when just using pickle for MatDescriptor

Thoughts?

@charlesbluca
Copy link
Member

Alternatively we could just drop this workaround and request users have at least CuPy >=8.0.0 installed.

Worth noting that with dask/dask#10161, we now make some attempt to test and document the minimum versions of various optional dependencies here; I intentionally kept that PR focused to CPU dependencies, but could be worth incorporating some form of mindeps testing into our GPU CI matrix so that we can document dependencies like this as well

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.

None yet

5 participants