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.

Sign up for GitHub

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

Jump to bottom

Use RMM's DeviceBuffer directly #235

Merged
merged 4 commits into from
Feb 7, 2020
Merged

Use RMM's DeviceBuffer directly #235

merged 4 commits into from
Feb 7, 2020

Conversation

jakirkham
Copy link
Member

This makes use of to_device and copy_to_host methods added to DeviceBuffer recently in PR ( rapidsai/rmm#268 ) to directly create and copy data into RMM or serialize from RMM back to host.

@jakirkham
Use DeviceBuffer to provide to_device function
64846f5 
This should bypass some overhead from Numba when constructing objects on
the device.
@jakirkham
Add a shim copy_to_host function
2d84f2f 
Try to use the object's own `copy_to_host` if available. Otherwise
fallback to coercing the object into a Numba `DeviceNDArray` and call
`copy_to_host` on it.
@jakirkham
Use the copy_to_host shim function
b5b2e9c
@jakirkham jakirkham requested a review from pentschev February 4, 2020 21:25
@jakirkham jakirkham requested a review from a team as a code owner February 4, 2020 21:25
@jakirkham
Copy link
Member Author

@pentschev, what sorts of data does host_to_device normally see? Am assuming here it is more-or-less bytes-like, but can handle other edge cases if needed. So please let me know if there is anything I should be aware of/addressing here 🙂

@pentschev
Copy link
Member

@jakirkham thanks for working on this.

what sorts of data does host_to_device normally see? Am assuming here it is more-or-less bytes-like, but can handle other edge cases if needed.

The host_to_device may not be bytes-like. I don't recall exactly how are all types transformed to, it depends on how each of the distributed CUDA serializers dump data, those that depend on this file https://github.com/dask/distributed/blob/master/distributed/protocol/cuda.py. So in general, I think we can expect all those types, probably not more than those today. Not sure if that answers your question though.

Also, it seems that tests are failing on deserialization.

@jakirkham
Copy link
Member Author

jakirkham commented Feb 4, 2020 •
edited
Loading

Thanks Peter! 😄

Alright will take a closer look at that. Thanks 🙂

Indeed. It looks like RMM 0.12.0 is being pulled in instead of the nightly. Not sure why that is. Do you know? (Edit: Asked ops offline)

@jakirkham
Copy link
Member Author

rerun tests

@jakirkham
Copy link
Member Author

Ok looks like RMM nightlies are now getting picked up. The remaining errors are legitimate will take a look at fixing these and update after.

@jakirkham
Flatten data and cast as uint8
cd77904 
Before sending the data to device, make sure to flatten it and cast it
as `uint8` so we can build a `DeviceBuffer` of it. From there we can
handle the remaining deserialization steps.
@codecov-io
Copy link

codecov-io commented Feb 6, 2020 •
edited by raydouglass
Loading

Codecov Report

Merging #235 into branch-0.13 will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.13     #235      +/-   ##
===============================================
+ Coverage        75.83%   75.93%   +0.10%     
===============================================
  Files               14       14              
  Lines              960      964       +4     
===============================================
+ Hits               728      732       +4     
  Misses             232      232

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a007e45...cd77904. Read the comment docs.

@jakirkham
Copy link
Member Author

Think I've fixed the CI issues. Could you please take another look, @pentschev? 🙂

@pentschev
Copy link
Member

LGTM, thanks @jakirkham for the PR!

@pentschev pentschev merged commit 869b790 into rapidsai:branch-0.13 Feb 7, 2020
@jakirkham jakirkham deleted the use_devbuf branch February 7, 2020 20:23
@jakirkham jakirkham mentioned this pull request Feb 8, 2020
Closed
@jakirkham jakirkham mentioned this pull request Mar 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pentschev pentschev pentschev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jakirkham @pentschev @codecov-io