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

Just use None for strides in DeviceBuffer #477

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

jakirkham
Copy link
Member

Originally there were some issues in CuPy and Numba with how they implemented __cuda_array_interface__ and handled other objects that supported it where they couldn't handle strides of None cleanly. So we added strides here with what it should be. However both libraries have since cleaned this up ( CuPy 6.6.0+, cupy/cupy#2669 and Numba 0.46.0+, numba/numba#4609 ) and we require newer versions of both. As some code contains fast paths when strides is None, switch to using None here as we ensure contiguous data in DeviceBuffers. This should allow us to take advantage of those fast paths where possible.

@jakirkham jakirkham requested a review from kkraus14 August 11, 2020 04:10
@jakirkham jakirkham requested a review from a team as a code owner August 11, 2020 04:10
@jakirkham
Copy link
Member Author

rerun tests

@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge Python Related to RMM Python API labels Aug 11, 2020
@kkraus14
Copy link
Contributor

Looks like something is segfaulting here which is a bit concerning. I'm guessing it's unrelated to this PR.

@jakirkham jakirkham force-pushed the use_contiguous_strides branch from aeadb9a to 06949e3 Compare August 11, 2020 05:27
Originally there were some issues in CuPy and Numba with how they
implemented `__cuda_array_interface__` and handled other objects that
supported it where they couldn't handle `strides` of `None` cleanly. So
we added `strides` here with what it should be. However both libraries
have since cleaned this up (CuPy 6.6.0+ and Numba 0.46.0+) and we
require newer versions of both. As some code contains fast paths when
`strides` is `None`, switch to using `None` here as we ensure contiguous
data in `DeviceBuffer`s. This should allow us to take advantage of those
fast paths where possible.
@jakirkham jakirkham force-pushed the use_contiguous_strides branch from 06949e3 to 0254df3 Compare August 11, 2020 05:29
@jakirkham
Copy link
Member Author

Yeah I've seen that before if the tests fail. Probably something not being cleaned up properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge Python Related to RMM Python API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants