Repository navigation
Allow public tuning of the cub::DeviceReduce::ReduceByKey and cub::DeviceRunLengthEncode::Encode - #9329
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. |
|
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:
important: WalkthroughPublic ReduceByKeyPolicy and RLEEncodePolicy types added; agent policy implementation moved to detail::agent_reduce_by_key_policy with deprecated alias; selectors, policy_hub, dispatch, kernels, DeviceRunLengthEncode, docs, benchmarks, and tests updated to new contracts. ChangesReduceByKey tuning API productization
Assessment against linked issues
suggestion:
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cub/cub/device/dispatch/tuning/tuning_reduce_by_key.cuh (1)
34-42: ⚡ Quick winsuggestion: Use repository Doxygen form for the new public policy type. This is a new public API surface, but the type comment is plain
//!text rather than the repository’s//!@brief`` style, and the field descriptions are only trailing comments. Converting this block to the standard Doxygen form will make the generated API docs consistent.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7dcce4ca-632f-4559-a542-3f091b2ec8f8
📒 Files selected for processing (11)
cub/benchmarks/bench/reduce/by_key.cucub/benchmarks/bench/run_length_encode/encode.cucub/cub/agent/agent_reduce_by_key.cuhcub/cub/device/device_reduce.cuhcub/cub/device/dispatch/dispatch_reduce_by_key.cuhcub/cub/device/dispatch/dispatch_streaming_reduce_by_key.cuhcub/cub/device/dispatch/tuning/tuning_reduce_by_key.cuhcub/cub/device/dispatch/tuning/tuning_rle_encode.cuhcub/test/catch2_test_device_reduce_by_key_custom_policy_hub.cucub/test/catch2_test_device_reduce_env.cucub/test/catch2_test_device_run_length_encode_env.cu
9db6c98 to
d00c348
Compare
This comment has been minimized.
This comment has been minimized.
d00c348 to
9ddc842
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cub/cub/device/dispatch/tuning/tuning_rle_encode.cuh (2)
36-37: ⚡ Quick winsuggestion: switch this doc line to Doxygen
//!@brief...format to match the repository documentation rule.As per coding guidelines, "Use Doxygen-style
//!comments with@brieftags for documentation."Source: Coding guidelines
61-67: ⚡ Quick winsuggestion: annotate the new host stream operator with
_CCCL_HOST_APIfor consistency with function annotation requirements in CUB headers.As per coding guidelines, "All functions must be marked with
_CCCL_HOST_API,_CCCL_DEVICE_API, or_CCCL_APIannotation."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5b3c0ab5-dba5-4e0c-8213-21f089580977
📒 Files selected for processing (13)
cub/benchmarks/bench/reduce/by_key.cucub/benchmarks/bench/run_length_encode/encode.cucub/cub/agent/agent_reduce_by_key.cuhcub/cub/device/device_reduce.cuhcub/cub/device/device_run_length_encode.cuhcub/cub/device/dispatch/dispatch_reduce_by_key.cuhcub/cub/device/dispatch/dispatch_streaming_reduce_by_key.cuhcub/cub/device/dispatch/tuning/tuning_reduce_by_key.cuhcub/cub/device/dispatch/tuning/tuning_rle_encode.cuhcub/test/catch2_test_device_reduce_by_key_custom_policy_hub.cucub/test/catch2_test_device_reduce_by_key_env_api.cucub/test/catch2_test_device_reduce_env.cucub/test/catch2_test_device_run_length_encode_env.cu
✅ Files skipped from review due to trivial changes (1)
- cub/cub/device/device_reduce.cuh
🚧 Files skipped from review as they are similar to previous changes (9)
- cub/cub/device/dispatch/dispatch_streaming_reduce_by_key.cuh
- cub/test/catch2_test_device_run_length_encode_env.cu
- cub/benchmarks/bench/run_length_encode/encode.cu
- cub/test/catch2_test_device_reduce_env.cu
- cub/test/catch2_test_device_reduce_by_key_custom_policy_hub.cu
- cub/benchmarks/bench/reduce/by_key.cu
- cub/cub/agent/agent_reduce_by_key.cuh
- cub/cub/device/dispatch/dispatch_reduce_by_key.cuh
- cub/cub/device/dispatch/tuning/tuning_reduce_by_key.cuh
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cub/test/catch2_test_device_run_length_encode_env.cu (1)
306-306: ⚡ Quick winsuggestion: Test category
"[reduce][device]"is inconsistent with all other tests in this file, which use"[run_length_encode][device]". Although RLEEncodePolicy is related to reduce-by-key, keeping tags uniform within a test file aids filtering and discoverability. Consider changing to"[run_length_encode][device]"or adding both tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7fa4eff3-bf1f-4427-80a8-a0c28da32845
📒 Files selected for processing (1)
cub/test/catch2_test_device_run_length_encode_env.cu
cub::DeviceReduce::ReduceByKeycub::DeviceReduce::ReduceByKey and cub::DeviceRunLengthEncode::Encode
🥳 CI Workflow Results🟩 Finished in 2h 26m: Pass: 100%/287 | Total: 10d 09h | Max: 2h 09m | Hits: 31%/676307See results here. |
Fixes: #9326
New public entities
cub::ReduceByKeyPolicyint threads_per_blockint items_per_threadBlockLoadAlgorithm load_algorithmCacheLoadModifier load_modifierBlockScanAlgorithm scan_algorithmLookbackDelayPolicy lookback_delaycub::RleEncodePolicyint threads_per_blockint items_per_threadBlockLoadAlgorithm load_algorithmCacheLoadModifier load_modifierBlockScanAlgorithm scan_algorithmLookbackDelayPolicy lookback_delay