[FEA] Add method to copy device_buffer back to host memory #219
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
This allows users to copy an `rmm::device_buffer` back to host memory given a pointer to the destination. It is up to the user to ensure they have allocated enough space to perform the copy and that the pointer is in fact on the host.
Make sure that we are able to use the `copy_to_host` method from Cython.
Allocate a `bytes` object that is uninitialized and then copy the data from the underlying `rmm::device_buffer` into the pointer held within `bytes`.
jakirkham
force-pushed
the
add_devbuf_tobytes
branch
from
4189071 to
3d29c34
Compare
harrism
requested changes
Dec 20, 2019
harrism
reviewed
Dec 20, 2019
kkraus14
reviewed
Dec 20, 2019
2 tasks
kkraus14
approved these changes
Jan 2, 2020
kkraus14
suggested changes
Jan 2, 2020
There was a problem hiding this comment.
Actually, @jakirkham would you mind adding some unit tests for this on the Python side?
harrism
requested changes
Jan 2, 2020
jrhemstad
requested changes
Jan 6, 2020
kkraus14
approved these changes
Jan 6, 2020
As we need to perform synchronization at the Cython level, ensure we can call `cudaStreamSynchronize` by externing it.
jakirkham
force-pushed
the
add_devbuf_tobytes
branch
2 times, most recently
from
0597262 to
f57934f
Compare
To ensure we have a valid `bytes` object with data in it, synchronize at the Cython level. Besides we no longer synchronize in C++. So we cannot rely on that any more.
jakirkham
force-pushed
the
add_devbuf_tobytes
branch
from
f57934f to
12a83af
Compare
Instead of taking a `cudaStream_t` typed `stream` value in `tobytes`, take a `uintptr_t` and cast it to `cudaStream_t` later. This way we can leverage Cython's knowledge of how to coerce Python objects to `uintptr_t`. Then it is a simple matter to cast the `uintptr_t` to `cudaStream_t` later.
jakirkham
force-pushed
the
add_devbuf_tobytes
branch
from
12a83af to
10601c9
Compare
jrhemstad
requested changes
Jan 7, 2020
|
Sorry for the churn. Think this is ready for another review @jrhemstad. Also @kkraus14 you may want to take another look as the Python side has changed a bit (though not dramatically). |
jrhemstad
reviewed
Jan 7, 2020
Co-Authored-By: Jake Hemstad <[email protected]>
jrhemstad
approved these changes
Jan 7, 2020
|
rerun tests (Conda package failed to download from server in one build) |
|
rerun tests (Only the |
|
rerun tests (auto-complete 🙄) |
kkraus14
reviewed
Jan 7, 2020
kkraus14
reviewed
Jan 7, 2020
kkraus14
suggested changes
Jan 7, 2020
kkraus14
approved these changes
Jan 7, 2020
|
Thanks all! 😄 |
jakirkham
commented
Jan 8, 2020
| * @param hb host allocated buffer to copy data to | ||
| * @param stream CUDA stream on which the device to host copy will be performed | ||
| *-------------------------------------------------------------------------**/ | ||
| void copy_to_host(const device_buffer& db, void* hb, cudaStream_t stream = 0) { |
There was a problem hiding this comment.
There were linking errors downstream as this was not declared inline and the function source code was not in a .cpp file. This has since been fixed with PR ( #230 ).
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.
Fixes #216
Adds the functionality to
rmm::device_bufferandrmm.DeviceBufferto copy data back from device memory to host memory. On the C++ side this copies data back to a user allocated buffer. On the Python side this copies data into a Pythonbytesobject.