Adding support for cupy.cuda.stream.ExternalStream #1559
Conversation
Cupy offers the `cupy.cuda.stream.ExternalStream` for utilizing external CUDA streams. Moreover, `cupy.cuda.get_current_stream()` will return an instance of `cupy.cuda.stream.ExternalStream` instead of `cupy.cuda.stream.Stream`, particularly when the current cuPy stream has been changed. Therefore, we must verify both types of instances to avoid errors. See details in the https://docs.cupy.dev/en/stable/user_guide/interoperability.html#cuda-stream-pointers
|
/ok to test |
|
Hi @harrism @miscco @jrhemstad, Would you kindly inform me if there are any additional steps required for this fix to be accepted? Thank you so much, |
wence-
added
non-breaking
|
/ok to test |
|
/ok to test |
There was a problem hiding this comment.
This looks fine to me. Thanks for the fix @lilohuang ! I would like someone more expert with cupy to review and approve before we merge, such as @leofang or @wence-.
|
/ok to test |
|
Can we add a test for this change? |
Agree, that's a good suggestion. However looking, we don't appear to have any tests of this |
|
/ok to test |
|
No need to block on tests if existing functionality is untested, but this is a gap that might be worth addressing in the future. |
|
/ok to test |
|
/merge |
|
Thanks @lilohuang! |
Description
Cupy offers the
cupy.cuda.stream.ExternalStreamfor utilizing external CUDA streams. Moreover,cupy.cuda.get_current_stream()will return an instance ofcupy.cuda.stream.ExternalStreaminstead ofcupy.cuda.stream.Stream, particularly when the current cuPy stream has been changed.Therefore, we must verify both types of instances to avoid errors.
See details in the https://docs.cupy.dev/en/stable/user_guide/interoperability.html#cuda-stream-pointers
Checklist