Repository navigation
Conversation
📝 Summary
Priority: Change: Other Merge Risk:
 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments: 1
docs/cudax/coop/group/mapping/group_as.rst: 1
58-58:Trivial · Maintainability & Code Quality · Quick win suggestion: Initialize
group_aswith an explicitRangeView.Use
cuda::std::views::all_t<decltype((unit_counts))>as the template argument:cudax::coop::group_as<cuda::std::views::all_t<decltype((unit_counts))>> m{unit_counts};Keep the deduction guide in the synopsis. The repository guidance applies to this documentation path and prohibits deduction-guide initialization. The comparable APIs explicitly document their deduction behavior, but
group_ashas no such exception.
-
-
- Source mode: Auto
- Repository: NVIDIA/cccl
- Branch: main
- File: .coderabbit.yaml
- Commit: 03c8d69
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
46da92a8-1245-4c31-a02e-b751208d9af2
-
docs/cudax/coop/group/mapping/group_as.rst
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
-
c558c86 to
4e4e31f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments: 1
docs/cudax/coop/group/mapping/group_as.rst: 1
55-58:Trivial · Maintainability & Code Quality · Quick win suggestion: Use explicit template arguments in the example.
The example relies on deduction guides for
cuda::std::arrayandgroup_as. Use explicit template arguments so the example follows the repository requirement and remains clear across supported toolchains.As per coding guidelines: do not rely on deduction guides for initialization.
Source: Coding guidelines
-
-
- Source mode: Auto
- Repository: NVIDIA/cccl
- Branch: main
- File: .coderabbit.yaml
- Commit: 03c8d69
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
cb16bc05-dcf2-4f2e-8551-fa4b92692a2d
-
cudax/include/cuda/experimental/coop/__group/mapping/group_as.cuhdocs/cudax/coop/group/mapping/group_as.rst
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
-
🥳 CI Workflow Results🟩 Finished in 2h 00m: Pass: 100%/73 | Total: 1d 03h | Max: 1h 14m | Hits: 32%/139368See results here. |
|
|
||
| The ``unit_counts`` range contains one size per subgroup. Units are assigned in order of their rank in the previous mapping result: the first subgroup gets the first ``unit_counts[0]`` units, the second gets the next ``unit_counts[1]`` units, and so on. If the previous mapping result describes multiple groups, the same sizes are applied independently to each group, and the resulting subgroup ranks are ordered first by the previous group rank, then by the index in ``unit_counts``. | ||
|
|
||
| ``RangeView`` must be a viewable sized range whose value type is convertible to ``cuda::std::uint32_t``. The stored view must support indexing and iteration through a const view. Counts must be nonnegative and representable as ``cuda::std::uint32_t``, and their sum and the resulting group count must also be representable in that type. All units in the same input group must agree on the sizes. |
There was a problem hiding this comment.
I might avoid listing the restrictions on the range in the documentation until you are very very certain what they are. It's easy for this to grow out of sync with the implementation.
| const cuda::std::array unit_counts{4, 12, 16}; | ||
|
|
||
| // Mapping that splits every group from the previous mapping result into 3 subgroups of 4, 12 and 16 units. | ||
| cudax::coop::group_as m{unit_counts}; |
There was a problem hiding this comment.
The example should show how this is used later on with some other code. Otherwise the reader is left thinking "so what?", because it's not clear where this fits in elsewhere.
No description provided.