Document and Migrate host_span vs span public APIs - #22588
Conversation
Clarified when to prefer `std::span` for host-side data and when to use `cudf::host_span`, including specific scenarios for each. This enhances the documentation for developers regarding memory management in the library.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConverts many host-side non-owning parameters from Changesstd::span migration and doc update
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
rapidsmpf links against libcudf and is pulled into CI prebuilt from nightlies. It's being compiled against the old host_span signatures. I created a new PR in rapidsmpf: rapidsai/rapidsmpf#1077 After merging rapidsmpf#1077, the next published rapidsmpf nightly should clear the failures in the current ci. |
PointKernel
left a comment
There was a problem hiding this comment.
Looks good and thank you @joannamooon for updating rapidsmpf accordingly.
There was a problem hiding this comment.
Those cuIO changes look legitimate to me. Just FYI, @mhaseeb123 is also working in the same files
|
/ok to test 0c4a9d4 |
|
@joannamooon I'm in the middle of a large refactor that will move all the parts of rapidsmpf that call libcudf into a new library in this repository. All rapidsmpf development is temporarily frozen as a result of that. Once that is done, though, you should have a much easier time getting unblocked here. |
…td-span # Conflicts: # cpp/src/io/parquet/experimental/hybrid_scan_impl.cpp # cpp/src/io/parquet/experimental/hybrid_scan_impl.hpp # cpp/src/io/parquet/io_utils/parquet_io_utils.cpp # cpp/src/rolling/grouped_rolling.cu
|
/ok to test 40581d5 |
|
/ok to test 210fb82 |
|
/ok to test 058fb95 |
|
/ok to test a10bd5c |
|
/ok to test c1834b7 |
|
/ok to test b3949e9 |
|
@joannamooon We don't need to keep the branch continuously updated with the latest upstream |
|
/ok to test c9a88c0 |
|
/merge |
Description
Closes #22560
DEVELOPER_GUIDE.mdcudf:: / cudf::detail::public APIs that used host_span purely as a host buffer view tostd::span(null mask, groupby, concatenate, interop, scalar, dictionaries, rolling, parquet, hybrid scan)span.pxd,hybrid_scan.pxd/.pyx) to exposestd::spanvia a newstd_span wrappercompute_groupby,compute_single_pass_aggs,extract_single_pass_aggs, etc.) were updated as a necessary cascade to keep the build consistent after the public API signatures changed — not as an intentional scope expansionChecklist