Add basic communicator concept#9426
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. |
|
/ok to test |
1 similar comment
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
Jacobfaib
force-pushed
the
jacobf/2026-06-10/communicator-concept
branch
from
6b75ba3 to
e68912d
Compare
|
/ok to test |
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
cudax/test/multi_gpu/concepts/communicator.cu (1)
37-43: ⚡ Quick winsuggestion:
test()on Line [37] is a compile-time helper and does not throw; addnoexceptfor consistency with the concept-check helper convention. Apply the same change tohas_all_reduce.cu(Line [42]) andhas_gather.cu(Line [35]) to keep this test suite uniform. As per coding guidelines, compile-time concept-check helpers should remainconstexprand non-throwing helpers should benoexcept.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4bf05b18-5d26-4234-8913-c4c9fd2b92e4
📒 Files selected for processing (17)
cudax/include/cuda/experimental/__multi_gpu/concepts.hcudax/test/CMakeLists.txtcudax/test/multi_gpu/CMakeLists.txtcudax/test/multi_gpu/concepts/CMakeLists.txtcudax/test/multi_gpu/concepts/collective_concepts_common.cuhcudax/test/multi_gpu/concepts/communicator.cucudax/test/multi_gpu/concepts/concepts_common.cuhcudax/test/multi_gpu/concepts/has_all_gather.cucudax/test/multi_gpu/concepts/has_all_reduce.cucudax/test/multi_gpu/concepts/has_all_to_all.cucudax/test/multi_gpu/concepts/has_all_to_all_v.cucudax/test/multi_gpu/concepts/has_broadcast.cucudax/test/multi_gpu/concepts/has_gather.cucudax/test/multi_gpu/concepts/has_gather_v.cucudax/test/multi_gpu/concepts/has_recv.cucudax/test/multi_gpu/concepts/has_reduce.cucudax/test/multi_gpu/concepts/has_send.cu
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.
Jacobfaib
requested review from
bernhardmgruber,
gevtushenko,
miscco and
pciolkosz
Jacobfaib
force-pushed
the
jacobf/2026-06-10/communicator-concept
branch
from
0009a44 to
c2bd580
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cudax/test/multi_gpu/concepts/has_all_reduce.cu (1)
28-38: 📐 Maintainability & Code Quality | ⚡ Quick winsuggestion: Template parameters
TpandOpshould use the reserved-identifier convention (_Tp,_Op) to match the style in the actual__has_all_reduceimplementation (which uses_Comm,_Ptr). Based on learnings, the guideline "use_Tp, not_T" indicates the underscore prefix is the preferred form for template parameters in cudax code.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 271f63bb-98c5-48fa-bce0-7227a1b7d398
📒 Files selected for processing (17)
cudax/include/cuda/experimental/__multi_gpu/concepts.hcudax/test/CMakeLists.txtcudax/test/multi_gpu/CMakeLists.txtcudax/test/multi_gpu/concepts/CMakeLists.txtcudax/test/multi_gpu/concepts/collective_concepts_common.cuhcudax/test/multi_gpu/concepts/communicator.cucudax/test/multi_gpu/concepts/concepts_common.cuhcudax/test/multi_gpu/concepts/has_all_gather.cucudax/test/multi_gpu/concepts/has_all_reduce.cucudax/test/multi_gpu/concepts/has_all_to_all.cucudax/test/multi_gpu/concepts/has_all_to_all_v.cucudax/test/multi_gpu/concepts/has_broadcast.cucudax/test/multi_gpu/concepts/has_gather.cucudax/test/multi_gpu/concepts/has_gather_v.cucudax/test/multi_gpu/concepts/has_recv.cucudax/test/multi_gpu/concepts/has_reduce.cucudax/test/multi_gpu/concepts/has_send.cu
🚧 Files skipped from review as they are similar to previous changes (15)
- cudax/test/multi_gpu/CMakeLists.txt
- cudax/test/multi_gpu/concepts/has_all_to_all.cu
- cudax/test/multi_gpu/concepts/has_reduce.cu
- cudax/test/multi_gpu/concepts/has_broadcast.cu
- cudax/test/multi_gpu/concepts/concepts_common.cuh
- cudax/test/multi_gpu/concepts/has_gather_v.cu
- cudax/test/multi_gpu/concepts/communicator.cu
- cudax/test/multi_gpu/concepts/has_gather.cu
- cudax/test/multi_gpu/concepts/has_all_to_all_v.cu
- cudax/test/multi_gpu/concepts/has_recv.cu
- cudax/test/multi_gpu/concepts/collective_concepts_common.cuh
- cudax/test/multi_gpu/concepts/has_all_gather.cu
- cudax/test/multi_gpu/concepts/has_send.cu
- cudax/include/cuda/experimental/__multi_gpu/concepts.h
- cudax/test/multi_gpu/concepts/CMakeLists.txt
This comment has been minimized.
This comment has been minimized.
Jacobfaib
force-pushed
the
jacobf/2026-06-10/communicator-concept
branch
from
c2bd580 to
14ebfba
Compare
🥳 CI Workflow Results🟩 Finished in 33m 37s: Pass: 100%/55 | Total: 4h 09m | Max: 33m 34s | Hits: 100%/34803See results here. |
2 participants
Description
closes
Checklist