Use cuda::stream_ref for core libcudf APIs - #23645
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
vyasr
added
3 - Ready for Review
ChatGPT Codex Connector
vyasr
marked this pull request as ready for review
August 13, 2026 02:39
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request migrates CUDA stream parameters from ChangesCUDA stream API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to The PR standardizes stream parameters across core libcudf APIs, but several related factory signatures and documentation examples still need follow-up to keep the public interface and generated documentation consistent. It is mergeable with explicit owner awareness of these bounded issues. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
cpp/include/cudf/column/column.hpp (1)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd non-default-stream coverage for the changed
cudf::columnAPIs.The existing tests and benchmarks use the default stream or omit the stream argument. Add tests and benchmark cases for all three APIs with a non-default
cuda::stream_ref, and validate dependent stream ordering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/include/cudf/column/column.hpp` at line 56, Add test and benchmark coverage for all three changed cudf::column APIs using a non-default cuda::stream_ref instead of relying only on the default stream or omitted argument. Verify results and dependent operations respect stream ordering, including synchronization or ordering checks appropriate to each API.Source: Coding guidelines
cpp/include/cudf/scalar/scalar_factories.hpp (1)
13-14: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse
cuda::stream_refformake_string_scalar.Update the declaration and definition to match the other scalar factories. Add explicit-stream unit-test coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/include/cudf/scalar/scalar_factories.hpp` around lines 13 - 14, Update make_string_scalar’s declaration and definition to accept cuda::stream_ref, matching the signatures of the other scalar factories and propagating the stream through its implementation. Add explicit-stream unit-test coverage for make_string_scalar.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/include/cudf/column/column_factories.hpp`:
- Line 17: Update the declarations and definitions of make_strings_column,
make_strings_column_batch, make_structs_column, and create_structs_hierarchy to
accept and propagate cuda::stream_ref instead of the legacy stream type. Extend
their related tests and benchmarks to exercise a non-default stream.
In `@cpp/include/cudf/contiguous_split.hpp`:
- Around line 158-159: Update the chunked_pack example to declare a
cuda::stream_ref variable, pass it before the memory-resource argument in
chunked_pack::create, and use stream.get() as the stream argument to
cudaMemcpyAsync.
In `@cpp/include/cudf/detail/scatter.hpp`:
- Around line 59-64: Correct the `@copydoc` target for the scatter declaration by
removing the extraneous bool parameter from the referenced cudf::detail::scatter
signature, so it matches the actual overload and resolves in Doxygen. Validate
the change with ./ci/checks/doxygen.sh.
In `@cpp/src/dictionary/replace.cu`:
- Around line 67-74: Update the `@copydoc` declaration above replace_nulls to
include the missing comma between the replacement column_view const& parameter
and cuda::stream_ref, matching the target signature and identifying the correct
overload. Run ./ci/checks/doxygen.sh to validate the documentation.
---
Nitpick comments:
In `@cpp/include/cudf/column/column.hpp`:
- Line 56: Add test and benchmark coverage for all three changed cudf::column
APIs using a non-default cuda::stream_ref instead of relying only on the default
stream or omitted argument. Verify results and dependent operations respect
stream ordering, including synchronization or ordering checks appropriate to
each API.
In `@cpp/include/cudf/scalar/scalar_factories.hpp`:
- Around line 13-14: Update make_string_scalar’s declaration and definition to
accept cuda::stream_ref, matching the signatures of the other scalar factories
and propagating the stream through its implementation. Add explicit-stream
unit-test coverage for make_string_scalar.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bc831aca-6ff4-44e3-93df-11d43244f2ae
📒 Files selected for processing (88)
cpp/include/cudf/column/column.hppcpp/include/cudf/column/column_device_view.cuhcpp/include/cudf/column/column_factories.hppcpp/include/cudf/column/column_stream.hppcpp/include/cudf/column/column_view.hppcpp/include/cudf/concatenate.hppcpp/include/cudf/contiguous_split.hppcpp/include/cudf/copying.hppcpp/include/cudf/detail/concatenate.hppcpp/include/cudf/detail/concatenate_masks.hppcpp/include/cudf/detail/contiguous_split.hppcpp/include/cudf/detail/copy.hppcpp/include/cudf/detail/copy_range.cuhcpp/include/cudf/detail/fill.hppcpp/include/cudf/detail/gather.hppcpp/include/cudf/detail/repeat.hppcpp/include/cudf/detail/scatter.cuhcpp/include/cudf/detail/scatter.hppcpp/include/cudf/detail/search.hppcpp/include/cudf/detail/sequence.hppcpp/include/cudf/dictionary/detail/concatenate.hppcpp/include/cudf/dictionary/detail/encode.hppcpp/include/cudf/dictionary/detail/merge.hppcpp/include/cudf/dictionary/detail/replace.hppcpp/include/cudf/dictionary/detail/search.hppcpp/include/cudf/dictionary/detail/update_keys.hppcpp/include/cudf/dictionary/dictionary_factories.hppcpp/include/cudf/dictionary/encode.hppcpp/include/cudf/dictionary/search.hppcpp/include/cudf/dictionary/update_keys.hppcpp/include/cudf/filling.hppcpp/include/cudf/partitioning.hppcpp/include/cudf/scalar/scalar.hppcpp/include/cudf/scalar/scalar_factories.hppcpp/include/cudf/search.hppcpp/include/cudf_test/column_utilities.hppcpp/src/column/column.cucpp/src/column/column_device_view.cucpp/src/column/column_factories.cppcpp/src/column/column_factories.cucpp/src/column/column_view.cppcpp/src/copying/concatenate.cucpp/src/copying/contiguous_split.cucpp/src/copying/copy.cppcpp/src/copying/copy.cucpp/src/copying/copy_range.cucpp/src/copying/gather.cucpp/src/copying/get_element.cucpp/src/copying/pack.cppcpp/src/copying/purge_nonempty_nulls.cucpp/src/copying/reverse.cucpp/src/copying/sample.cucpp/src/copying/scatter.cucpp/src/copying/segmented_shift.cucpp/src/copying/shift.cucpp/src/copying/slice.cucpp/src/copying/split.cppcpp/src/dictionary/add_keys.cucpp/src/dictionary/decode.cucpp/src/dictionary/detail/concatenate.cucpp/src/dictionary/detail/merge.cucpp/src/dictionary/dictionary_factories.cucpp/src/dictionary/encode.cucpp/src/dictionary/match_keys.cucpp/src/dictionary/remove_keys.cucpp/src/dictionary/replace.cucpp/src/dictionary/search.cucpp/src/dictionary/set_keys.cucpp/src/filling/calendrical_month_sequence.cucpp/src/filling/fill.cucpp/src/filling/repeat.cucpp/src/filling/sequence.cucpp/src/partitioning/partitioning.cucpp/src/partitioning/round_robin.cucpp/src/scalar/scalar.cppcpp/src/scalar/scalar_factories.cppcpp/src/search/contains_column.cucpp/src/search/contains_scalar.cucpp/src/search/contains_table.cucpp/src/search/contains_table_impl.cucpp/src/search/contains_table_impl.cuhcpp/src/search/search_ordered.cucpp/tests/column/column_device_view_test.cucpp/tests/column/column_test.cppcpp/tests/copying/concatenate_tests.cppcpp/tests/copying/shift_tests.cppcpp/tests/streams/column_view_test.cppcpp/tests/utilities/column_utilities.cu
| #include <rmm/cuda_stream_view.hpp> | ||
|
|
||
| #include <cuda/std/utility> | ||
| #include <cuda/stream_ref> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline cpp/include/cudf/column/column_factories.hpp --items all --type function
rg -n -C 3 \
'rmm::cuda_stream_view|cuda::stream_ref|make_strings_column|make_structs_column|create_structs_hierarchy' \
cpp/include/cudf/column/column_factories.hpp \
cpp/src/column/column_factories.cpp \
cpp/src/column/column_factories.cuRepository: NVIDIA/cudf
Length of output: 22503
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- declarations and definitions ---'
rg -n -C 5 \
'make_strings_column\s*\(|make_strings_column_batch\s*\(|make_structs_column\s*\(|create_structs_hierarchy\s*\(' \
cpp --glob '*.{cpp,cu,hpp,cuh,h,cc,cxx}' \
| head -n 500
printf '%s\n' '--- stream type usage in factory-related implementation files ---'
rg -n -C 3 \
'rmm::cuda_stream_view|cuda::stream_ref' \
cpp/src cpp/include/cudf/column \
--glob '*.{cpp,cu,hpp,cuh,h,cc,cxx}' \
| head -n 700
printf '%s\n' '--- changed paths and focused diff summary ---'
git diff --stat -- cpp/include/cudf/column/column_factories.hpp cpp/src
git status --shortRepository: NVIDIA/cudf
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused header ---'
sed -n '350,590p' cpp/include/cudf/column/column_factories.hpp
printf '%s\n' '--- factory definitions outside tests ---'
rg -n \
'(^|::)(make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy)\s*\(' \
cpp/src --glob '*.{cpp,cu,cc,cxx}' \
--glob '!**/tests/**'
printf '%s\n' '--- stream-bearing factory declarations and definitions ---'
rg -n \
'(make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy)[^(]*\(|rmm::cuda_stream_view|cuda::stream_ref' \
cpp/include/cudf/column/column_factories.hpp cpp/src/column \
--glob '*.{hpp,h,cuh,cpp,cu,cc,cxx}'
printf '%s\n' '--- focused diff ---'
git diff -- cpp/include/cudf/column/column_factories.hpp cpp/src/columnRepository: NVIDIA/cudf
Length of output: 21596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- public string factory definitions ---'
sed -n '150,235p' cpp/src/strings/strings_column_factories.cu
printf '%s\n' '--- public struct factory definitions ---'
rg -n -C 8 \
'std::unique_ptr<.*(make_structs_column|create_structs_hierarchy)|make_structs_column\(|create_structs_hierarchy\(' \
cpp/src --glob '*.{cpp,cu,cc,cxx}' \
--glob '!**/tests/**' \
| rg -v '(^|/)(make_|create_).*:' | head -n 300
printf '%s\n' '--- all implementation stream-view declarations near factory definitions ---'
rg -l \
'make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy' \
cpp/src --glob '*.{cpp,cu,cc,cxx}' \
--glob '!**/tests/**' \
| while read -r file; do
if rg -q 'rmm::cuda_stream_view|cuda::stream_ref' "$file"; then
printf '%s\n' "--- $file"
rg -n 'rmm::cuda_stream_view|cuda::stream_ref|make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy' "$file"
fi
done
printf '%s\n' '--- focused non-default-stream tests ---'
sed -n '1,110p' cpp/tests/streams/strings/factory_test.cpp
rg -n -C 4 \
'make_structs_column|create_structs_hierarchy|make_strings_column_batch|make_strings_column\(' \
cpp/tests/streams cpp/tests/column \
--glob '*.{cpp,cu,cc,cxx,hpp,h}' \
| head -n 350Repository: NVIDIA/cudf
Length of output: 26743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- stream test helpers and string factory tests ---'
rg -n -C 6 \
'get_default_stream\(|get_new_stream\(|cuda_stream_view|stream_ref' \
cpp/tests/streams cpp/include/cudf_test \
--glob '*.{cpp,cu,hpp,h,cuh}' \
| head -n 500
printf '%s\n' '--- factory benchmarks ---'
rg -n -C 4 \
'make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy|factory' \
cpp/benchmarks cpp/benchmarks --glob '*.{cpp,cu,hpp,h,cuh}' \
2>/dev/null | head -n 400
printf '%s\n' '--- all stream-specific factory tests ---'
rg -n -C 5 \
'make_strings_column|make_strings_column_batch|make_structs_column|create_structs_hierarchy' \
cpp/tests/streams cpp/tests/column/factories_test.cpp \
--glob '*.{cpp,cu,hpp,h,cuh}' \
| head -n 500
printf '%s\n' '--- header stream includes ---'
sed -n '1,35p' cpp/include/cudf/column/column_factories.hppRepository: NVIDIA/cudf
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- string stream tests ---'
cat -n cpp/tests/streams/strings/factory_test.cpp
printf '%s\n' '--- string factory benchmark stream setup ---'
sed -n '1,100p' cpp/benchmarks/string/make_strings_column.cu
printf '%s\n' '--- factory tests with explicit stream arguments ---'
rg -n -C 3 \
'get_default_stream|get_new_stream|cudaStream|stream[[:space:]]*[),]' \
cpp/tests/column/factories_test.cpp cpp/tests/structs/structs_column_tests.cpp \
--glob '*.{cpp,cu,hpp,h,cuh}'
printf '%s\n' '--- stream helper definition ---'
rg -n -C 5 \
'get_default_stream\s*\(' \
cpp/include/cudf_test cpp/tests \
--glob '*.{hpp,h,cpp,cu,cuh}' \
| head -n 120
printf '%s\n' '--- struct factory benchmarks ---'
rg -l \
'make_structs_column|create_structs_hierarchy' \
cpp/benchmarks --glob '*.{cpp,cu,hpp,h,cuh}' \
| sortRepository: NVIDIA/cudf
Length of output: 5932
Migrate the remaining stream-bearing factory APIs.
Use cuda::stream_ref in the declarations and definitions of make_strings_column, make_strings_column_batch, make_structs_column, and create_structs_hierarchy. Add non-default-stream coverage to the related tests and benchmarks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/include/cudf/column/column_factories.hpp` at line 17, Update the
declarations and definitions of make_strings_column, make_strings_column_batch,
make_structs_column, and create_structs_hierarchy to accept and propagate
cuda::stream_ref instead of the legacy stream type. Extend their related tests
and benchmarks to exercise a non-default stream.
Source: Coding guidelines
Comment on lines
+158
to
159
| cuda::stream_ref stream = cudf::get_default_stream(), | ||
| rmm::device_async_resource_ref temp_mr = cudf::get_current_device_resource_ref()); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file=$(git ls-files 'cpp/include/cudf/contiguous_split.hpp')
printf '%s\n' "$file"
ast-grep outline "$file"
printf '\n--- relevant documentation and declarations ---\n'
rg -n -C 12 'chunked_pack|cudaMemcpyAsync|contiguous_split|create\(' "$file"
printf '\n--- related declarations/usages ---\n'
rg -n -C 8 'chunked_pack::create|class chunked_pack|struct chunked_pack|create\(.*user_buffer_size|cudaMemcpyAsync' cpp/include cpp/src tests 2>/dev/null | head -n 240Repository: NVIDIA/cudf
Length of output: 26307
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- stream_ref definition and handle accessors ---'
rg -n -C 10 'class stream_ref|struct stream_ref|stream_ref::get|get\(\).*cudaStream_t|cudaStream_t.*get\(\)' cpp/include cpp/src 2>/dev/null | head -n 220
printf '%s\n' '--- all stream declarations in the example/header ---'
rg -n -C 4 '\bstream\b|cuda::stream_ref' cpp/include/cudf/contiguous_split.hpp
printf '%s\n' '--- exact API argument types ---'
sed -n '143,228p' cpp/include/cudf/contiguous_split.hpp
printf '%s\n' '--- implementation forwarding ---'
sed -n '2197,2210p' cpp/src/copying/contiguous_split.cuRepository: NVIDIA/cudf
Length of output: 8995
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
p = Path("cpp/include/cudf/contiguous_split.hpp")
s = p.read_text()
decl = re.search(
r'static std::unique_ptr<chunked_pack> create\(\s*'
r'cudf::table_view const& input,\s*'
r'std::size_t user_buffer_size,\s*'
r'cuda::stream_ref stream\s*=.*?,\s*'
r'rmm::device_async_resource_ref temp_mr\s*=.*?\);',
s,
re.S,
)
assert decl, "chunked_pack::create declaration was not found"
print("create declaration orders arguments as: input, user_buffer_size, stream, temp_mr")
example = s[s.index(" * `@code`{.pseudo}", s.index("class chunked_pack")):
s.index(" * `@endcode`", s.index("class chunked_pack"))]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
print("example passes mr as the third create argument and stream as cudaMemcpyAsync's final argument")
print("example declares stream:", bool(re.search(r'\b(stream|auto const stream)\b', example)))
PY
printf '%s\n' '--- repository references to stream_ref handle access ---'
rg -n -C 4 'cuda::stream_ref|stream\.get\(\)|stream\.value\(\)' cpp include 2>/dev/null | head -n 260Repository: NVIDIA/cudf
Length of output: 336
🌐 Web query:
CCCL cuda::stream_ref get() CUDA runtime handle interoperability
💡 Result:
In the CUDA Core Compute Libraries (CCCL), cuda::stream_ref is a non-owning wrapper designed to interoperate seamlessly with native CUDA Runtime (cudart) handles [1][2]. The get member function is the standard mechanism for retrieving the underlying native cudaStream_t handle from a cuda::stream_ref object [3][2]. Key details regarding this interoperability include: 1. Retrieval: Calling.get on a cuda::stream_ref returns the managed cudaStream_t handle, allowing you to pass the underlying stream to traditional CUDA Runtime APIs [1][3]. 2. Construction: You can construct a cuda::stream_ref directly from a native cudaStream_t handle [3][2]. Furthermore, cudaStream_t supports implicit conversion to cuda::stream_ref, facilitating easier integration into C++ codebases [3][4]. 3. Lifetime Management: Because cuda::stream_ref is non-owning, it does not manage the lifetime of the underlying stream; the caller must ensure that the stream handle remains valid for the duration of the cuda::stream_ref's usage [3][4]. 4. Ownership Transfer: For scenarios requiring ownership (where the C++ object should manage the stream's lifetime, such as destroying it upon destruction), use the owning counterpart, cuda::stream [1][2]. The owning type supports from_native_handle to take control of a raw handle and release to relinquish that control back to the native CUDA Runtime [1][5][4]. This pattern is consistent across other CCCL runtime types, such as event_ref, providing a unified approach to bridging modern C++ interfaces with legacy CUDA Runtime code [1][4].
Citations:
- 1: https://nvidia.github.io/cccl/unstable/libcudacxx/runtime/cudart_interactions.html
- 2: https://nvidia.github.io/cccl/unstable/libcudacxx/runtime/stream.html
- 3: https://nvidia.github.io/cccl/unstable/libcudacxx/api/classcuda_1_1stream__ref.html
- 4: https://developer.nvidia.com/blog/cccl-runtime-a-modern-c-runtime-for-cuda/
- 5: https://nvidia.github.io/cccl/unstable/libcudacxx/api/structcuda_1_1stream.html
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
s = Path("cpp/include/cudf/contiguous_split.hpp").read_text()
start = s.index(" * `@code`{.pseudo}", s.index("class chunked_pack"))
end = s.index("`@endcode`", start)
example = s[start:end]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
assert not re.search(r"auto\s+(?:const\s+)?stream\s*=", example)
print("The example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly as cudaMemcpyAsync's final argument.")
create = re.search(
r'static std::unique_ptr<chunked_pack> create\(\s*'
r'cudf::table_view const& input,\s*'
r'std::size_t user_buffer_size,\s*'
r'cuda::stream_ref stream\s*=.*?,\s*'
r'rmm::device_async_resource_ref temp_mr\s*=.*?\);',
s,
re.S,
)
assert create
print("The declaration requires the third argument to be cuda::stream_ref and the fourth to be temp_mr.")
PYRepository: NVIDIA/cudf
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
s = Path("cpp/include/cudf/contiguous_split.hpp").read_text()
start = s.index(" * `@code`{.pseudo}")
end = s.index("`@endcode`", start)
example = s[start:end]
assert "auto mr = cudf::get_current_device_resource_ref();" in example
assert "auto chunked_packer = cudf::chunked_pack::create(tv, user_buffer_size, mr);" in example
assert "cudaMemcpyDefault,\n * stream);" in example
assert not re.search(r"auto\s+(?:const\s+)?stream\s*=", example)
print("The example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly as cudaMemcpyAsync's final argument.")
decl_start = s.index("static std::unique_ptr<chunked_pack> create(")
decl_end = s.index(");", decl_start)
decl = s[decl_start:decl_end]
assert re.search(r"cuda::stream_ref\s+stream", decl)
assert re.search(r"rmm::device_async_resource_ref\s+temp_mr", decl)
print("The declaration requires stream before temp_mr.")
PYRepository: NVIDIA/cudf
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
lines = Path("cpp/include/cudf/contiguous_split.hpp").read_text().splitlines()
start = next(i for i, line in enumerate(lines) if "`@code`{.pseudo}" in line)
end = next(i for i in range(start + 1, len(lines)) if "`@endcode`" in lines[i])
print("\n".join(f"{i+1}: {lines[i]}" for i in range(start, end + 1)))
print("--- checks ---")
checks = {
"mr declaration": any("auto mr = cudf::get_current_device_resource_ref();" in x for x in lines[start:end]),
"create call": any("create(tv, user_buffer_size, mr);" in x for x in lines[start:end]),
"cudaMemcpyDefault": any("cudaMemcpyDefault," in x for x in lines[start:end]),
"direct stream": any("stream);" in x for x in lines[start:end]),
"stream declaration": any("auto const stream" in x or "auto stream" in x for x in lines[start:end]),
}
for name, value in checks.items():
print(f"{name}: {value}")
PYRepository: NVIDIA/cudf
Length of output: 609
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
lines = Path("cpp/include/cudf/contiguous_split.hpp").read_text().splitlines()
marker = next(i for i, line in enumerate(lines) if "Create a table_view" in line)
start = max(i for i in range(marker + 1) if "`@code`{.pseudo}" in lines[i])
end = next(i for i in range(marker, len(lines)) if "`@endcode`" in lines[i])
example = lines[start:end]
assert any("auto mr = cudf::get_current_device_resource_ref();" in x for x in example)
assert any("create(tv, user_buffer_size, mr);" in x for x in example)
assert any("cudaMemcpyDefault," in x for x in example)
assert any("stream);" in x for x in example)
assert not any("auto const stream" in x or "auto stream" in x for x in example)
decl_start = next(i for i, line in enumerate(lines) if "static std::unique_ptr<chunked_pack> create(" in line)
decl = "\n".join(lines[decl_start:decl_start + 6])
stream_pos = decl.index("cuda::stream_ref stream")
mr_pos = decl.index("rmm::device_async_resource_ref temp_mr")
assert stream_pos < mr_pos
print("The chunked_pack example has no stream declaration.")
print("The example passes mr as create's third argument.")
print("The example passes stream directly to cudaMemcpyAsync.")
print("chunked_pack::create declares stream before temp_mr.")
PYRepository: NVIDIA/cudf
Length of output: 359
Correct the chunked_pack example for cuda::stream_ref.
Declare stream, pass it before mr to chunked_pack::create, and pass stream.get() to cudaMemcpyAsync.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/include/cudf/contiguous_split.hpp` around lines 158 - 159, Update the
chunked_pack example to declare a cuda::stream_ref variable, pass it before the
memory-resource argument in chunked_pack::create, and use stream.get() as the
stream argument to cudaMemcpyAsync.
Source: Coding guidelines
Comment on lines
59
to
64
| /** | ||
| * @copydoc cudf::detail::scatter(table_view const&,column_view const&,table_view | ||
| * const&,bool,rmm::cuda_stream_view,rmm::device_async_resource_ref) | ||
| * const&,bool,cuda::stream_ref,rmm::device_async_resource_ref) | ||
| * | ||
| * @throws cudf::logic_error if `scatter_map` span size is larger than max of `size_type`. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the @copydoc overload signature.
Line 61 adds bool, but the referenced cudf::detail::scatter overload at Lines 53-57 has no bool parameter. Doxygen cannot resolve this overload target. Remove bool from the signature.
As per coding guidelines, “In order to run doxygen as a linter on C++/CUDA code, run ./ci/checks/doxygen.sh.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/include/cudf/detail/scatter.hpp` around lines 59 - 64, Correct the
`@copydoc` target for the scatter declaration by removing the extraneous bool
parameter from the referenced cudf::detail::scatter signature, so it matches the
actual overload and resolves in Doxygen. Validate the change with
./ci/checks/doxygen.sh.
Source: Coding guidelines
Comment on lines
67
to
74
| /** | ||
| * @copydoc cudf::dictionary::detail::replace_nulls(cudf::column_view const&,cudf::column_view | ||
| * const& rmm::cuda_stream_view, rmm::device_async_resource_ref) | ||
| * const& cuda::stream_ref, rmm::device_async_resource_ref) | ||
| */ | ||
| std::unique_ptr<column> replace_nulls(dictionary_column_view const& input, | ||
| dictionary_column_view const& replacement, | ||
| rmm::cuda_stream_view stream, | ||
| cuda::stream_ref stream, | ||
| rmm::device_async_resource_ref mr) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the @copydoc parameter list.
Line 69 lacks a comma between the second const& and cuda::stream_ref. The target signature does not identify the overload declared at Lines 71-74. Change it to const&, cuda::stream_ref.
As per coding guidelines, “In order to run doxygen as a linter on C++/CUDA code, run ./ci/checks/doxygen.sh.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/src/dictionary/replace.cu` around lines 67 - 74, Update the `@copydoc`
declaration above replace_nulls to include the missing comma between the
replacement column_view const& parameter and cuda::stream_ref, matching the
target signature and identifying the correct overload. Run
./ci/checks/doxygen.sh to validate the documentation.
Source: Coding guidelines
bdice
approved these changes
Aug 13, 2026
bdice
left a comment
bdice
left a comment
Contributor
There was a problem hiding this comment.
All looks good. The CodeRabbit comments are worth fixing, but maybe in a followup.
ttnghia
approved these changes
Aug 13, 2026
Contributor
Author
Agreed, I'll either pull these out into a standalone PR or if they're obviously related I'll add them into the next PR in the |
Contributor
Author
|
/merge |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
This second batch migrates core libcudf column, scalar, copying, dictionary, filling, search, and partitioning APIs and tests from
rmm::cuda_stream_viewtocuda::stream_ref. It leaves default stream helpers and owning RMM stream types unchanged for later batches.Contributes to #23636
Checklist