Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a NCCL sub-communicator using ncclCommSplit #2495

Open
wants to merge 10 commits into
base: branch-25.04
Choose a base branch
from

Conversation

seunghwak
Copy link
Contributor

NCCL expects to create a sub-communicator using ncclCommSplit (like we create a MPI sub-communicator using MPI_Comm_split) but raft currently creates a sub-communicator from scratch using ncclCommInitRank on a subset of GPUs.

This first prevents sharing resources among the global communicator and sub-communicators. We also experienced erroneous sub-communicator behaviors when we have a non-power-of-two number of GPUs.

This PR updates raft to create NCCL sub-communicators using ncclCommSplit.

@seunghwak seunghwak requested a review from a team as a code owner November 14, 2024 18:56
@github-actions github-actions bot added the cpp label Nov 14, 2024
@seunghwak seunghwak added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 14, 2024
@seunghwak seunghwak self-assigned this Nov 14, 2024
@seunghwak seunghwak changed the title [WIP] Create a NCCL sub-communicator using ncclCommSplit Create a NCCL sub-communicator using ncclCommSplit Nov 14, 2024
@cjnolet cjnolet changed the base branch from branch-24.12 to branch-25.02 January 28, 2025 20:27
@cjnolet
Copy link
Member

cjnolet commented Jan 28, 2025

@seunghwak is this PR ready for review? Is this something you'd like to include in the 25.02 release? I Just changed the base branch accordingly. Hopefully I didn't mess anything up.

@seunghwak
Copy link
Contributor Author

seunghwak commented Feb 8, 2025

Yes, this PR is ready to review but it seems like too late for 25.02, let's target 25.04

@seunghwak seunghwak changed the base branch from branch-25.02 to branch-25.04 February 8, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants