Handle sliced struct/list columns properly in concatenate() bounds checking. #8760
Conversation
nvdbaranec
added
bug
| : cudf::detail::get_value<offset_type>( | ||
| scv.offsets(), scv.offset() + b.size(), stream) - | ||
| cudf::detail::get_value<offset_type>(scv.offsets(), scv.offset(), stream)); |
There was a problem hiding this comment.
Each get_value implicitly calls stream sync. So we have 2 syncs here. I wonder if we need to manually call cudaMemcpy twice, then justs do 1 stream sync for better performance.
|
rerun tests |
galipremsagar
changed the title
galipremsagar
changed the title
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-21.08 #8760 +/- ##
===============================================
Coverage ? 10.51%
===============================================
Files ? 116
Lines ? 18962
Branches ? 0
===============================================
Hits ? 1993
Misses ? 16969
Partials ? 0 Continue to review full report at Codecov.
|
| thrust::fill(rmm::exec_policy(), | ||
| offsets->mutable_view().begin<offset_type>(), | ||
| offsets->mutable_view().end<offset_type>(), | ||
| list_size); | ||
| thrust::exclusive_scan(rmm::exec_policy(), | ||
| offsets->view().begin<offset_type>(), | ||
| offsets->view().end<offset_type>(), | ||
| offsets->mutable_view().begin<offset_type>()); |
There was a problem hiding this comment.
You could use std::fill and std::inclusive_scan to fill out a host memory buffer and then cudaMemcpy to the offsets column in order to avoid changing this to .cu file.
There was a problem hiding this comment.
The numbers in some of these cases are pretty big. I switched to thrust for speed purposes.
|
rerun tests |
|
@gpucibot merge |
Fixes #8748
Note:
concatenate_tests.cppwas renamed toconcatenate_tests.cubecause of the addition of some thrust calls.Existing overflow tests moved to
OverflowTestsection. New tests specific to this PR are:Overflowtest.PreslicedOverflowTest.BigColumnsSmallSlices