Skip to content

[diffusion] feat: capture-safe pynccl all-to-all - #33775

Merged
mickqian merged 1 commit into
sgl-project:mainfrom
mickqian:mick/pynccl-capture-a2a
Aug 6, 2026
Merged

mickqian merged 1 commit into
sgl-project:mainfrom
mickqian:mick/pynccl-capture-a2a

Conversation

@mickqian

@mickqian mickqian commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Motivation

A CUDA graph cannot hold a torch.distributed ProcessGroupNCCL collective: its host-side per-op bookkeeping advances once at capture and never again, so on replay the ranks stop agreeing about which collective is in flight and hang. This is the reason whole-forward DiT capture (#31852) must refuse sharded runs today, and it is not a NCCL limitation — srt already runs collectives inside captured regions by calling raw nccl through its pynccl wrapper, which carries no per-call host state. The diffusion tree's vendored pynccl lacked both an all-to-all and the capture-window switch.

Modifications

  • pynccl_wrapper.py: bind ncclGroupStart / ncclGroupEnd.
  • pynccl.py: all_to_all_single built from grouped ncclSend/ncclRecv (equal and uneven splits, matching dist.all_to_all_single semantics), plus change_state so the communicator is live only inside a capture window rather than becoming a mode. A disabled communicator raises instead of silently no-oping, since a silent skip would return whatever the output buffer already held.
  • test_pynccl_a2a_capture_2_gpu.py, registered in the 2-gpu suite: bitwise parity with dist.all_to_all_single eagerly, then capture once and replay against three fresh inputs, then assert the disabled state refuses.

Nothing consumes this in the serving path yet: routing the sequence-parallel all-to-all through it during whole-forward capture is the follow-up on top of #31852, which is what lets --dit-cuda-graph full run sharded (verified end-to-end there: capture succeeds at ulysses 2 and 4, outputs bitwise-identical to eager).

Test

4×H200, world=2 and world=4: eager result bitwise-equal to dist.all_to_all_single; captured graph replays correctly for three fresh inputs (PYNCCL_A2A_CAPTURE world=2 PASS, world=4 PASS). The three files here are byte-identical to the branch those runs used; the 2-GPU case is the CI guard.

Checklist

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ✅ Run #31076290463
Latest PR Test (Extra): ❌ Run #31076289925

@github-actions github-actions Bot added the diffusion SGLang Diffusion label Aug 6, 2026
@mickqian

mickqian commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci label Aug 6, 2026
"""
if enable is None:
enable = self.available
old_disabled = self.disabled

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

self.disabled is False after successful initialization, so this context restores it to False on exit. The final disabled-communicator check in the new test will therefore execute the all-to-all and fail. Should the communicator be disabled before entering the capture window, or should the test set that state explicitly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Right on both counts — and this caught a real verification gap: the transport was probe-verified on GPUs, but this test file itself had not run there (the devbox was reclaimed before it could). This communicator initializes enabled, so the check now sets the state explicitly with change_state(enable=False), and the whole file has now actually run on 2xH200: PYNCCL_A2A_CAPTURE PASS.

if stream is None:
stream = current_stream()
chunk = input_.numel() // self.world_size
send_counts = input_split_sizes or [chunk] * self.world_size

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dist.all_to_all_single defines split sizes along dim 0, but this treats them as flattened element counts and validates them against numel(). For an input shaped (4, 3) with splits [1, 3], PyTorch sends 3 and 9 elements, while this code asserts 4 == 12. Could we either restrict this API to 1-D flattened tensors or convert dim-0 splits to element counts and add an uneven 2-D parity test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed — splits now convert dim-0 rows to element counts (sum validated against size(0)), so n-D tensors split identically to dist.all_to_all_single, and the test gained an uneven 2-D parity case compared bitwise against the dist result. Passing on 2xH200.

@mickqian
mickqian force-pushed the mick/pynccl-capture-a2a branch from 181cd9f to ea243a1 Compare August 6, 2026 03:24
Review caught both: split sizes were treated as flattened element counts where
dist.all_to_all_single defines them along dim 0 -- rows now convert to element
counts so n-D tensors split identically to torch, with an uneven 2-D parity
case in the test. And this communicator initializes enabled, so the
raise-when-disabled check now sets its state explicitly instead of relying on
change_state's exit value.

Verified on 2xH200: the full test file passes (bitwise parity, capture+replay,
uneven 2-D splits, disabled refusal).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mickqian
mickqian force-pushed the mick/pynccl-capture-a2a branch from ea243a1 to e0680ca Compare August 6, 2026 06:08
@mickqian
mickqian merged commit bfce378 into sgl-project:main Aug 6, 2026
124 of 136 checks passed
kfhfar pushed a commit to kfhfar/sglang that referenced this pull request Aug 7, 2026
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Xia-Weiwen pushed a commit to Xia-Weiwen/sglang that referenced this pull request Aug 10, 2026
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Atituiset pushed a commit to Atituiset/sglang that referenced this pull request Sep 10, 2026
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion SGLang Diffusion run-ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants