Repository navigation
Segmented scan tests to use cuda buffer - #11374
oleksandr-pavlyk wants to merge 28 commits into
Conversation
f2a2931 to
df5f8d7
Compare
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Buffer operations and comparison c2h/include/c2h/buffer_utils.cuh, c2h/include/c2h/catch2_test_helper.h, c2h/include/c2h/checked_memory_resource.cuh, c2h/include/c2h/device_and_stream.h, c2h/include/c2h/buffer_generators.cuh, cub/test/catch2_test_c2h_buffers.cu |
Adds device-to-host copy and comparison helpers, bounded mismatch reporting, initialized host-buffer creation, customizable device buffers, and buffer tests. |
Checked uniform-offset generation c2h/include/c2h/generator_common.h, c2h/include/c2h/vector_generators.h, c2h/generators_uniform_offsets.cu, c2h/include/c2h/buffer_generators.cuh, cub/test/catch2_test_c2h_buffers.cu |
Validates offset counts and destination capacity. Adds stream-based generation and resizable device-buffer and paired-buffer APIs. |
Generator and shared test support c2h/generators_gen_values.cu, c2h/include/c2h/detail/generators.cuh, cub/test/catch2_test_scan_reference.cuh, cub/test/catch2_test_device_scan.cuh, cub/test/catch2_test_numeric_helpers.cuh, cub/test/catch2_test_device_reduce.cuh |
Moves value-generation implementation into the detail header. Adds shared scan references and numeric helpers for extended, custom, and vector types. |
Segmented-scan test migration
| Layer / File(s) | Summary |
|---|---|
Test utilities and reference functions cub/test/catch2_test_device_segmented_scan_utils.cuh, cub/test/catch2_test_scan_reference.cuh, cub/test/catch2_test_device_scan.cuh |
Adds scan test type aliases, host-buffer generation and copy helpers, and shared exclusive and inclusive scan references. |
Core scan tests cub/test/catch2_test_device_segmented_scan.cu |
Uses CUDA host/device buffers and explicit streams for scan calls and result checks. |
API and environment tests cub/test/catch2_test_device_segmented_scan_api.cu, cub/test/catch2_test_device_segmented_scan_env.cu, cub/test/catch2_test_device_segmented_scan_env_api.cu |
Uses CUDA buffers and stream-bearing environments for scan, tuning, queued setup, overload resolution, and result comparisons. |
Multi-segment and noncommutative tests cub/test/catch2_test_device_segmented_scan_multi_segment.cu, cub/test/catch2_test_device_segmented_scan_noncommutative.cu |
Uses explicit streams and CUDA buffers for dispatch, test data, expected results, and output checks. |
Launch-helper stream support
| Layer / File(s) | Summary |
|---|---|
Stream-aware launch helpers and tests cub/test/catch2_test_launch_helper.h, cub/test/catch2_test_launch_wrapper.cu, cub/test/catch2_test_memory_resources.h, docs/cub/developer/test_overview.rst |
Updates host, graph, device, and environment launch paths to handle streams and stream-associated buffers. Adds stream-forwarding tests and documents launch behavior. The device-side memory resource now supports a null allocation counter. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Migrate segmented-scan tests to CUDA buffer containers with checked memory resources instead of C2H host and device vectors [11373] | ✅ |
Possibly related issues
- [Task] Make entities defined by DECLARE_LAUNCH_WRAPPER stream aware #11346: The PR adds stream-like argument handling to launch wrappers and uses explicit streams in segmented-scan test setup and execution.
- Change segmented scan tests to use cuda::buffer #10897: The PR migrates segmented-scan tests to CUDA host and device buffers.
Priority:
Change: Refactor
Merge Risk: 
b7b08
-
The reviewed changes preserve buffer, stream, and graph-capture behavior without introducing a concrete correctness or availability risk.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
df5f8d7 to
2103b66
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
2103b66 to
452f928
Compare
This comment has been minimized.
This comment has been minimized.
452f928 to
9d76f67
Compare
This comment has been minimized.
This comment has been minimized.
9d76f67 to
9adb174
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
114d31b to
0173bae
Compare
This comment has been minimized.
This comment has been minimized.
- Migrated all six segmented-scan test suites to cuda::device_buffer/cuda::host_buffer
created using c2h::make_device_buffer and c2h::make_host_buffer which used
checked allocator memory resources
- Added shared CUDA and segmented-scan test utilities
- Added stream-aware launch-helper support and coverage
- Added uniform-offset buffer generators and pre-allocation size validation
- Updated C2H generator tests and CUB testing documentation
…rage Track the total mismatch count while retaining only the first and last five mismatches needed for diagnostics. This avoids allocating temporary host storage proportional to the compared range size in segmented-scan tests. The new implementation keeps only the information printed on failure: - The first five mismatches go into first_mismatches. - The most recent five go into last_mismatches. - Additional mismatches overwrite the oldest trailing entry using a circular index. - A final rotation restores chronological order. - total_mismatches is still counted exactly. - The three good values preceding the first mismatch are still retained. For example, with mismatches at indices 0–11, the result stores: First: 0, 1, 2, 3, 4 Last: 7, 8, 9, 10, 11 Total: 12 The diagnostic output remains the same, but auxiliary mismatch storage changes from being proportional to the input size—O(N)—to a fixed maximum of ten records, O(1). This matters because segmented-scan tests can compare very large buffers close to the configured memory limit.
Require equal vector sizes in addition to zero element mismatches. This prevents an empty actual vector from matching a nonempty expected vector. Also added a regression test covering an empty actual vector against a nonempty expected vector in cub/test/catch2_test_c2h_buffers.cu
…opies finish This could be reverted once NVIDIA#11958 lands
Added tests for passing owning cuda::stream argument.
bernhardmgruber
left a comment
bernhardmgruber
left a comment
There was a problem hiding this comment.
Blocking, until above feedback is addressed.
This is still flat, all changes together, but it should be much closer to the desired shape and validates that it all sits together before I start splitting it all up
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments: 5
c2h/include/c2h/buffer_utils.cuh: 1
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Complete the documented function contracts. Add
//! @param[in/out/in,out]entries for every parameter ofcopy_to_hostandread_single. Add//! @returnforread_single. As per coding guidelines, “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Also applies to: 87-87
Source: Coding guidelines
c2h/include/c2h/checked_memory_resource.cuh: 1
234-234: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Complete the new overload’s documentation. Add
//! @param[in]entries forstream,device,num_items, andvalue. Add//! @returnfor the initialized host buffer. As per coding guidelines, “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Source: Coding guidelines
c2h/include/c2h/device_and_stream.h: 2
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Make
default_stream_refconstexpr. The function only constructs a null handle. Itscuda::stream_refconstructor isconstexpr. (raw.githubusercontent.com) As per coding guidelines, “Useconstexprfor functions that do not depend on run-time features, such as pointers.”Source: Coding guidelines
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: Use the required function documentation format. Replace the plain comment with
//! @briefand add//! @returnfor the stream reference. As per coding guidelines, “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Source: Coding guidelines
cub/test/catch2_test_device_segmented_scan_env_api.cu: 1
28-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftsuggestion: Use public CUDA APIs in the extracted examples.
The documentation extracts these regions. The markers include
c2h::make_device_buffer, while thedevicedeclaration is outside the markers. Define the device inside each example and usecuda::make_device_bufferfor the documented setup. Do not move the buffers outside the markers because the extracted scan calls use them.
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: NVIDIA/cccl
- Branch: main
- File: .coderabbit.yaml
- Commit: 7bf3883
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
c879fbb5-9245-4c0f-9d3b-e489749b7575
📒 Files selected for processing: 16
c2h/generators_gen_values.cuc2h/generators_uniform_offsets.cuc2h/include/c2h/buffer_utils.cuhc2h/include/c2h/checked_memory_resource.cuhc2h/include/c2h/detail/generators.cuhc2h/include/c2h/device_and_stream.hcub/test/catch2_test_c2h_buffers.cucub/test/catch2_test_device_segmented_scan.cucub/test/catch2_test_device_segmented_scan_api.cucub/test/catch2_test_device_segmented_scan_env.cucub/test/catch2_test_device_segmented_scan_env_api.cucub/test/catch2_test_device_segmented_scan_multi_segment.cucub/test/catch2_test_device_segmented_scan_noncommutative.cucub/test/catch2_test_device_segmented_scan_utils.cuhcub/test/catch2_test_launch_helper.hcub/test/catch2_test_launch_wrapper.cu
💤 Files with no reviewable changes: 1
- c2h/generators_gen_values.cu
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
custom_type_t<Policies> can be padded so we need to fully initialize all bytes of this type before filling it up with a `min` value.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🧹 Nitpick comments: 4
c2h/include/c2h/buffer_generators.cuh: 1
51-52:Trivial · Maintainability & Code Quality · Quick win suggestion: Restore the required function documentation tags. These changed comments document functions with plain
//text. Add//! @briefand a//! @param[in/out/in,out]entry for each parameter. Add//! @returnto the non-void functions. As per coding guidelines: “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Also applies to: 78-80, 91-93, 113-113, 121-123, 141-143, 156-158, 176-178
Source: Coding guidelines
c2h/include/c2h/buffer_utils.cuh: 1
60-61:Trivial · Maintainability & Code Quality · Quick win suggestion: Document these helpers with the required API tags. The new comments document
copy_to_hostandread_singlewithout//! @briefor parameter entries.read_singlealso needs//! @return. As per coding guidelines: “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Also applies to: 90-91
Source: Coding guidelines
c2h/include/c2h/detail/current_device.cuh: 1
12-15:Trivial · Maintainability & Code Quality · Quick win suggestion: Keep
assert_current_devicedocumentation in the required format. Replace the plain comments with//! @briefand//! @param[in] device. As per coding guidelines: “Documented functions must include//! @brief,//! @param[in/out/in,out]for every parameter, and//! @returnfor non-void functions.”Source: Coding guidelines
c2h/include/c2h/device_and_stream.h: 1
30-30:Trivial · Maintainability & Code Quality · Quick win suggestion: Add the required tags to
default_stream_refdocumentation. Its plain comments document a non-void function. Use//! @briefand//! @return. As per coding guidelines: “Documented functions must include//! @brief,//! @param[in/out,in,out]for every parameter, and//! @returnfor non-void functions.”Source: Coding guidelines
-
-
- Source mode: Auto
- Repository: NVIDIA/cccl
- Branch: main
- File: .coderabbit.yaml
- Commit: 7bf3883
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
55d06e4f-67e5-43e1-97f0-b236ef2c0742
-
c2h/include/c2h/buffer_generators.cuhc2h/include/c2h/buffer_utils.cuhc2h/include/c2h/checked_memory_resource.cuhc2h/include/c2h/detail/current_device.cuhc2h/include/c2h/device_and_stream.hcub/test/catch2_test_c2h_buffers.cucub/test/catch2_test_device_segmented_scan_api.cucub/test/catch2_test_launch_helper.hcub/test/catch2_test_launch_wrapper.cu
-
- c2h/include/c2h/checked_memory_resource.cuh
- cub/test/catch2_test_device_segmented_scan_api.cu
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 7h 06m: Pass: 100%/187 | Total: 8d 23h | Max: 2h 56m | Hits: 39%/538559See results here. |
|
This PR has been split into smaller, more focused PRs. The PR to change non-API tests for Closing this PR now. |
Description
This PR contains the final portion split from #10898.
It migrates the six CUB segmented-scan test suites from
c2h::host_vectorandc2h::device_vectortocuda::host_bufferandcuda::device_bufferbacked by C2H checked memory resources.CUB segmented-scan implementation is not changed.
The changes in this PR:
cuda::device_bufferandcuda::host_buffer.C2H_CONFIG_MAIN.closes #11373
gated by #11344Checklist