Skip to content

Nccl all reduce#1226

Merged
nathanielsimard merged 84 commits intomainfrom
nccl-all-reduce
Mar 18, 2026
Merged

Nccl all reduce#1226
nathanielsimard merged 84 commits intomainfrom
nccl-all-reduce

Conversation

@Charles23R
Copy link
Contributor

@Charles23R Charles23R commented Mar 12, 2026

Validate your PR with burn.

It is important that you make sure that you don't introduce any bugs in burn.

Instructions

  • Create a new branch or fork of the burn repo
  • Update the main Cargo.toml with this PR hash.
  • Fix any broken tests or compilation errors in burn.
  •  Submit a PR in burn with your fixes and link it here.

Burn PR : tracel-ai/burn#4640

@Charles23R Charles23R marked this pull request as draft March 17, 2026 14:32
@Charles23R Charles23R marked this pull request as ready for review March 17, 2026 19:52
Comment on lines +37 to +43
let guard = self.lock.lock.lock();
let map = guard.map.borrow();
let state = map.get(&TypeId::of::<S>()).unwrap();
let utilities = state.utilities.clone();
core::mem::drop(map);
core::mem::drop(guard);
utilities
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the self.lock, this is not mutable.

Comment on lines +281 to +284
assert_eq!(
src.stream, dst.stream,
"Source and destination should be on the same stream."
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of panic, I would add an error to each stream and return.

@nathanielsimard nathanielsimard merged commit 20585bb into main Mar 18, 2026
5 checks passed
@nathanielsimard nathanielsimard deleted the nccl-all-reduce branch March 18, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants