search

rapidsai / cudf

cuDF - GPU DataFrame Library
https://docs.rapids.ai/api/cudf/stable/
Apache License 2.0
8.37k stars 894 forks source link

[FEA] Iterator versions of make_device_uvector_sync() and make_device_uvector_async() #10453

Open nvdbaranec opened 2 years ago

nvdbaranec commented 2 years ago

Ran into some code in a PR that was staging results into a std::vector just so that it could be passed to a factory function. Seems like it would be useful to have versions that take (device) iterators directly.

github-actions[bot] commented 2 years ago

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

GregoryKimball commented 2 years ago

This reminds me of the string column factory discussion in #5150. @nvdbaranec would you please post a snippet showing what you have in mind?

github-actions[bot] commented 2 years ago

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

nvdbaranec commented 2 years ago

This reminds me of the string column factory discussion in #5150. @nvdbaranec would you please post a snippet showing what you have in mind?

Late reply. Snippet would pretty much be exactly like David's example in your link, 

template <typename Iterator>
rmm::device_uvector<T> make_device_uvector_async(
  Iterator values_begin, Iterator values_end,
  rmm::cuda_stream_view stream        = cudf::default_stream_value,
  rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource())
davidwendt commented 2 years ago

Would you be able to find any examples you mention in the description? Are you saying device data is being copied into a std::vector to build a device_uvector?

nvdbaranec commented 2 years ago

No, just that we have a lot of cases where the input is expressed as an iterator that the user has to manually put in a std::vector before calling the host_span overload. If you could just pass the iterators and let the function do the staging it would cut down on some clutter.

I believe the specific example was this, but it's pretty common in the code.

https://github.com/rapidsai/cudf/blob/03f1c1c5c5fcf90bd594aabd41b6e15f54690777/java/src/main/native/src/row_conversion.cu#L1842

nvdbaranec commented 2 years ago

Although I guess I did specify (device) in the description. Those would be useful as well, though maybe only applicable in limited circumstances.

davidwendt commented 2 years ago

Sorry, this does not make sense to me. The iterator is for host data then? There is no shortcut here. The data would need to be copied to a host vector (i.e. std::vector) before creating a device_uvector. The iterator version does not help solve this fact.