Tuning policy cleanup 1/2#9710
Conversation
|
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:
📝 WalkthroughWalkthroughDevice policy Catch2 tests were renamed to a properties naming scheme, and several tests now verify ChangesPolicy property test updates
Possibly related PRs
Suggested reviewers: Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cub/test/catch2_test_device_segmented_sort_keys_env.cu (1)
613-663: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winsuggestion: no
STATIC_REQUIRE(p1 == p2)(or partitioning_threshold-specific check) is added alongside the new per-sub-policy comparisons. The component checks don't verify the top-levelSegmentedSortPolicyaggregate equality/inequality operator itself.diff
STATIC_REQUIRE(p1_medium == p2_medium); STATIC_REQUIRE_FALSE(p1_medium != p2_medium); + + STATIC_REQUIRE(p1 == p2); + STATIC_REQUIRE_FALSE(p1 != p2);Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7733a74a-f8a2-4126-960e-d8ac82405dd5
📒 Files selected for processing (20)
cub/test/catch2_test_device_adjacent_difference_env.cucub/test/catch2_test_device_find_bound_sorted_values_env.cucub/test/catch2_test_device_find_env.cucub/test/catch2_test_device_for_env.cucub/test/catch2_test_device_histogram_env.cucub/test/catch2_test_device_memcpy_env.cucub/test/catch2_test_device_merge_env.cucub/test/catch2_test_device_merge_sort_env.cucub/test/catch2_test_device_partition_env.cucub/test/catch2_test_device_radix_sort_env.cucub/test/catch2_test_device_reduce_env.cucub/test/catch2_test_device_run_length_encode_env.cucub/test/catch2_test_device_scan_by_key_env.cucub/test/catch2_test_device_scan_env.cucub/test/catch2_test_device_segmented_radix_sort_env.cucub/test/catch2_test_device_segmented_reduce_env.cucub/test/catch2_test_device_segmented_scan_env.cucub/test/catch2_test_device_segmented_sort_keys_env.cucub/test/catch2_test_device_select_env.cucub/test/catch2_test_device_transform_env.cu
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cub/test/catch2_test_device_segmented_reduce_env.cu (1)
648-650: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuesuggestion: unscoped enumerators (
cub::BLOCK_REDUCE_WARP_REDUCTIONS,cub::LOAD_LDG) here vs. scopedcub::BlockReduceAlgorithm::.../cub::CacheLoadModifier::...in the reduce test. Harmless but inconsistent within the same cohort.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1e01a70b-781d-4707-9548-e3ed2a081365
📒 Files selected for processing (3)
cub/test/catch2_test_device_reduce_env.cucub/test/catch2_test_device_segmented_radix_sort_env.cucub/test/catch2_test_device_segmented_reduce_env.cu
🚧 Files skipped from review as they are similar to previous changes (1)
- cub/test/catch2_test_device_segmented_radix_sort_env.cu
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cub/test/catch2_test_device_adjacent_difference_env.cu (1)
275-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: this
to_stringlambda +REQUIRE(!to_string(p1).empty())pattern is duplicated verbatim across ~9 policy test files in this PR stack. Consider hoisting it into a shared test header (e.g. arequire_streamable_nonempty(p)helper) to avoid maintaining N copies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 33425755-662a-4616-af59-05307a84af03
📒 Files selected for processing (20)
cub/test/catch2_test_device_adjacent_difference_env.cucub/test/catch2_test_device_find_bound_sorted_values_env.cucub/test/catch2_test_device_find_env.cucub/test/catch2_test_device_for_env.cucub/test/catch2_test_device_histogram_env.cucub/test/catch2_test_device_memcpy_env.cucub/test/catch2_test_device_merge_env.cucub/test/catch2_test_device_merge_sort_env.cucub/test/catch2_test_device_partition_env.cucub/test/catch2_test_device_radix_sort_env.cucub/test/catch2_test_device_reduce_env.cucub/test/catch2_test_device_run_length_encode_env.cucub/test/catch2_test_device_scan_by_key_env.cucub/test/catch2_test_device_scan_env.cucub/test/catch2_test_device_segmented_radix_sort_env.cucub/test/catch2_test_device_segmented_reduce_env.cucub/test/catch2_test_device_segmented_scan_env.cucub/test/catch2_test_device_segmented_sort_keys_env.cucub/test/catch2_test_device_select_env.cucub/test/catch2_test_device_transform_env.cu
🚧 Files skipped from review as they are similar to previous changes (11)
- cub/test/catch2_test_device_find_env.cu
- cub/test/catch2_test_device_merge_env.cu
- cub/test/catch2_test_device_memcpy_env.cu
- cub/test/catch2_test_device_segmented_reduce_env.cu
- cub/test/catch2_test_device_segmented_sort_keys_env.cu
- cub/test/catch2_test_device_transform_env.cu
- cub/test/catch2_test_device_select_env.cu
- cub/test/catch2_test_device_segmented_scan_env.cu
- cub/test/catch2_test_device_partition_env.cu
- cub/test/catch2_test_device_scan_env.cu
- cub/test/catch2_test_device_radix_sort_env.cu
This comment has been minimized.
This comment has been minimized.
88011d7 to
4cf753f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
6fd5701 to
61ff6c1
Compare
This comment has been minimized.
This comment has been minimized.
61ff6c1 to
739345c
Compare
This comment has been minimized.
This comment has been minimized.
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 9h 17m: Pass: 100%/242 | Total: 1d 08h | Max: 1h 08m | Hits: 99%/149992See results here. |
ReducePolicyoperator<<Fixes part of: #9378