ENH: Simplify CuPy asarray and to_device
#314
Open
+25
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
crusaderky
commented
Apr 18, 2025
| elif not isinstance(device, _Device): | ||
| raise ValueError(f"Unsupported device {device!r}") | ||
| else: | ||
| # see cupy/cupy#5985 for the reason how we handle device/stream here |
There was a problem hiding this comment.
Issue fixed in 2021
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies CuPy’s asarray and to_device functions by streamlining the handling of the copy parameter and device context, and by updating test exclusions accordingly.
- Removed outdated test references regarding copy=False from cupy-xfails.txt
- Updated the asarray function in cupy/_aliases.py to accept copy as None and handle it via cp.asarray
- Revised device and stream handling in _helpers.py to use direct type checks against cp.cuda.Device and cp.cuda.Stream
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cupy-xfails.txt | Removed xfail tests related to copy=False implementation |
| array_api_compat/cupy/_aliases.py | Simplified asarray implementation and removed legacy copy handling and device context management |
| array_api_compat/common/_helpers.py | Updated device and stream handling in to_device |
Comments suppressed due to low confidence (1)
array_api_compat/common/_helpers.py:791
- The updated type check for device using cp.cuda.Device is concise; however, verify that this check fully replaces the earlier logic and correctly handles all valid device cases in production.
if not isinstance(device, cp.cuda.Device):
crusaderky
commented
Apr 18, 2025
array_api_compat/cupy/_aliases.py
Outdated
Comment on lines
93
to
94
| if not copy and res is not obj: | ||
| raise ValueError("Unable to avoid copy while creating an array as requested") |
There was a problem hiding this comment.
This is less than ideal as it causes a brief spike in memory usage, but IMHO it's better than outright failing
crusaderky
force-pushed
the
cupy_asarray
branch
from
d922875 to
8c31248
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Salvaged from #293
Simplify to_device and asarray for cupy and implement copy=False