Repository navigation
Sequence parallel: one captured all-to-all for Ulysses; drop the all-gather exchange - #897
Conversation
…ange CommGroup.all_to_all sends one packed buffer with all_to_all_single instead of a list of split pieces that it then concatenated, and only the collective is hidden from torch.compile, so the packing and unpacking stay in the caller's graph. It replays from a CUDA graph, so ulysses_attention loses prefer_all_gather and its all-gather exchange, which moved P times the bytes, and Cosmos3's captured denoise uses the all-to-all like its eager path. ulysses_attention takes the sequence and head dims, for batched layouts. The exchange is from LTX-2.5's SP work (#376), which kept its own copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Automated review🟡 No findings on Description
Questions
What the review checked
Advisory, not a merge gate. Rules: |
Gaurav-Shah05
left a comment
There was a problem hiding this comment.
Thanks for the follow-up, the shape is right, but the captured single-request t2i got slower on H100, the opposite of the description's numbers.
On 2 H100s with configs/cosmos3_nano_sp2.yaml and your capture settings, each build booted once on each GPU pair, median of 8 requests after 2 warmups, with images identical to main:
| main | this PR | Δ | |
|---|---|---|---|
| t2i 320x192 (captured) | 0.69 s | 0.82 s | 1.19x ↑ |
| t2i 832x480 (captured) | 1.07 s | 1.14 s | 1.06x ↑ |
| t2i 1280x720 (eager) | 3.88 s | 3.62 s | 1.07x ↓ |
| c=1 throughput, 832x480 | 0.90 req/s | 0.86 req/s | 1.04x ↓ |
- The slowdown isn't the exchange. Alone in a CUDA graph, the packed all-to-all beats main's all-gather path (3.1 against 5.1 ms per forward at 320x192), so the extra time is elsewhere in the captured step.
- The 1280x720 gain. The description credits the gains to dropping the captured all-gather, but this size is over
graph_max_latent_areaon SP2, so both builds run it eager, where main already used the all-to-all. Its gain comes from sending one packed buffer instead.
|
|
||
| One ``all_to_all_single`` over a packed buffer, which replays from a | ||
| CUDA graph. Only the collective is hidden from torch.compile: the | ||
| packing and unpacking stay in the caller's graph, where inductor |
There was a problem hiding this comment.
With the torch.compiler.disable on _all_to_all_single, dynamo compiles all_to_all as a frame of its own, so the packing and unpacking end up in separate graphs, not in the caller's.
In my runs both ways of keeping the collective in the caller's graph, deleting the disable or wrapping it in a torch.library.custom_op as ar_fusion.py does, made the served SP2 images wrong.
So the disable is needed.
There was a problem hiding this comment.
Agreed, and confirmed: compiling a caller on gloo gives four graphs (producer, packing, unpacking, consumer), so the docstring's claim was wrong. I also tried the one variant you hadn't, _functional_collectives.all_to_all_single with no disable: one graph, and bit-exact against full attention in isolation under compile + capture on 2×H200 (even and uneven), but served Cosmos3 SP2 hit an illegal memory access in FlashInfer's prefill during the bs=8 capture warm-up on all three boots. So the disable stays; 3110916 corrects the docstring and the description says what the boundary costs.
| if even_scatter and even_gather: | ||
| # [.., N, ..] -> [P, .., N/P, ..]: piece r goes to rank r, and the | ||
| # piece received from rank r has the same shape | ||
| send = input_.unflatten(scatter_dim, (world_size, -1)) |
There was a problem hiding this comment.
This is a bug for negative dims. On two ranks, scatter_dim=-2 or gather_dim=-3 returns a wrong tensor with no error, where main returned the right one. unflatten adds a dim, so movedim(scatter_dim, 0) moves the piece's own dim instead of the rank dim, and movedim(0, gather_dim) goes wrong the same way. It should be fixed if all_to_all normalize both dims first, as all_gather does.
There was a problem hiding this comment.
Thanks, reproduced on two gloo ranks: all four negative combinations wrong on this branch, all right on main. Fixed in 3110916 by normalizing both dims first, as all_gather does; the exchange tests now run with negative dims too, and four of them fail without the fix. No in-tree caller passed negative dims, so served output was unaffected.
| # The 2D (TP x SP) captured denoise graph hit a CUDA illegal access at | ||
| # 480p+ when the Ulysses exchange was an all-gather, composed with the TP | ||
| # all-reduce (pure-TP and pure-SP capture cleanly; only the combination at | ||
| # this scale). Not yet re-checked on the all-to-all exchange. For 2D, |
There was a problem hiding this comment.
With #705 under it and this cap removed, configs/cosmos3_nano_tp2_sp2.yaml captures both 192x320 and 480x832 on 4 H100s. The images match the SP2 ones at 38 and 41 dB PSNR, so the two_d cap and this comment can go.
…dary costs `unflatten` adds a dim before the `movedim`s, so a negative `scatter_dim` or `gather_dim` pointed at the wrong one and the exchange returned a wrong tensor without an error (main's list form handled them). Both are made positive first, as `all_gather` does; the exchange tests now also run with negative dims, and four of them fail without this. The docstring said the packing stays in the caller's compiled graph. It doesn't: with the collective behind `torch.compiler.disable`, dynamo compiles `all_to_all` as its own frame. Tracing the collective instead (`_functional_collectives.all_to_all_single`) keeps one graph and is bit-exact in isolation, but served Cosmos3 SP2 hit an illegal memory access in the batched capture's warm-up, so the disable stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The cap to the smallest tier was for an illegal access seen with the all-gather exchange composed with the TP all-reduce. With the all-to-all, `cosmos3_nano_tp2_sp2` on 4x H100 (with #705 for startup) captures 192x320 and 480x832 and matches the SP2 images at 38 and 41 dB PSNR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A freed ephemeral port can be taken by another xdist worker before the spawned ranks bind it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dghosef
left a comment
There was a problem hiding this comment.
pushed some minor test / doc changes, lgtm once you resolve with Gaurav
The exchanges are latency-bound at small sizes: on Cosmos3 SP2 at 320x192 a point-to-point all-to-all call took ~14 us against ~8 us for the all-gather it replaced, ~7200 times a request. q, k and v now go in one exchange: each rank's share of the three tensors' heads is laid side by side, so scattering the heads sends rank r all of its share, and the received tensor splits back into q, k and v. Different query and key/value head counts work, as long as each divides by the group size, which Ulysses already requires. Cosmos3-Nano SP2, 2x H200, single request (median of 8 after 2 warm-ups), alternated with the previous head, three boots each: 320x192 0.633 -> 0.548 s, 832x480 0.927 -> 0.871 s, 1280x720 (eager) 3.44 -> 2.78 s; images byte-identical across all six boots, 600 more requests (single and 8 at a time) without a failure. Compiled and captured alone, bit-exact against one rank attending everything, 100 -> 84 us at 960 tokens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iguous Ulysses packed q, k and v interleaved by head and handed the result to all_to_all, which packed it again: a second full copy, and q, k and v came back as non-contiguous slices that LTX's attention copies once more. On LTX SP2 that was ~2% slower than separate exchanges. all_to_all now takes several tensors that differ only in the scattered extent. Rank r's block of the send buffer holds its piece of each tensor one after another, written with one copy per tensor, and each tensor unpacks on its own, so the copies are those of separate exchanges in one collective. Ulysses passes (q, k, v). 2x H200, two boots per side, alternated: - LTX-2.5 SP2 against separate exchanges: 19.52 / 21.72 / 22.30 videos/min at c=1/2/4 against 19.55 / 21.79 / 22.24, two-stage 5.78 / 6.31 against 5.77 / 6.35; video and audio byte-identical. - Cosmos3-Nano SP2 against the interleaved version, single request: 320x192 0.552 vs 0.544 s, 832x480 0.872 vs 0.866 s, 1280x720 2.760 vs 2.764 s, so it keeps that version's gain over separate exchanges; images byte-identical, 400 more requests without a failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I profiled the captured step on H200 (nsys with CUDA-graph node tracing), base 0b1a723 against 538cbd7, per 320×192 request:
So q, k and v now go in one exchange (6eb3f3c):
The Cosmos3 rows come from three boots each, measured with the earlier one-exchange layout (6f298da). The final layout measured level with it on Cosmos3; it replaced it because it is level with separate exchanges on LTX, where the earlier layout was ~2% slower. The LTX row is the final layout, two boots each. Images and videos are byte-identical throughout, and 1,000 more Cosmos3 requests (single and 8 at a time) completed without a failure. If you can rerun your H100 comparison on 6eb3f3c, that would show whether the slowdown is gone there. I'll also run it on |
Live test: #897 on 2× H100 (Gaurav's H100 slowdown)Cosmos3-Nano,
Idle time between graphs is about the same on all three (286–301 ms per 320×192 request), so the extra graph breaks cost nothing visible.
|
|
@NSagan271 Thanks for the profile and the live test. I ran them as well and the slowdown is gone at the head. You were also right that it was the exchange, not something else in the step as I said before. I also ran the 4-GPU configs, and SP4 with batched captures gives wrong images. With I tried to pin down the cause and It isn't your exchange, though.
I'm opening a PR that turns it off by default on CUDA workers. With it, this head's SP4 images match main's exactly and the SP2 speedups hold, so this can go in right after it. I will post the numbers in the description there. Let me know your thoughts. |
@Gaurav-Shah05 thanks for hunting down the bug! Yes, lmk when you make a PR and I can review it. I guess the long-term solution would be to make sure the exchange buffers actually land in the right memory locations, but setting |
With NCCL registering the all-to-all buffers inside CUDA graphs, Cosmos3 SP4 with batched captures gave wrong images, on #897's exchange and on its first version once its buffers moved; with registration off every variant matched main.
|
@Gaurav-Shah05 merged the PR! |
|
For the long term, do you think it's better to look for what breaks inside NCCL's registration rather than where the buffers live? the allocator can put them anywhere, and some placements just trigger it. |
Gaurav-Shah05
left a comment
There was a problem hiding this comment.
good for merge from my end
Yeah, I think so. Lmk if you want me to take a look? |
Main brought (since 6cfe39c): #892 Parakeet and Nemotron ASR with the transducer as a graph loop (and the shared mrope_position_ids helper), #917 pytest 9.2 flag proxies, #897 one captured all-to-all for Ulysses, #915 NCCL graph registration off by default, #836 Qwen3-Omni Thinker prefix caching (ARNodeSubmodule.cuts_tensor_inputs, marks_dynamic_dims, the conductor's _streamed_partitions), #909 bench cells, #900 Command A+ batched image tiles. Conflict: - mstar/model/submodule_base.py: main's ARNodeSubmodule.cuts_tensor_inputs beside the stack's token_stop and its stop checks. Independent class attributes; both kept. Ported onto the stack's mechanisms: - mstar/model/parakeet/submodules.py: check_stop_batched drops the step_walk parameter (the stack's three-argument signature) and reads its "active" flags through HostRows.first_values instead of its own row-map loop. Its stop is a flag, not a token, so it has no TokenStop. - test/modular/test_transducer_loop.py: the call without step_walk. The conductor's _streamed_partitions (#836) sits beside _pick_replicas / _register_request; it reads only partition_states and _resource_specs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings #858's merge of main: #892 Parakeet / Nemotron ASR, #917, #897 Ulysses all-to-all, #915, #836 Qwen3-Omni Thinker prefix caching, #909, #900. Conflicts: - docs/adding_models.rst: main's marks_dynamic_dims sentence and #859's compile_regions paragraph are independent facts; both kept, in one paragraph. - mstar/model/qwen3_omni/submodules.py (3 hunks, plus auto-merged hunks in the same class): AudioEncoderSubmodule. Two mechanisms for one cause (the HF encoder recompiling per clip length): #859 runs it eagerly (disable_torch_compile) and packs a step's clips into calls of at most MAX_BATCH_FRAMES; #836 builds the encoder's chunk tensors in prepare_inputs with mark_dynamic so the per-clip forward compiles once (marks_dynamic_dims, _max_seqlen). One mechanism: #859's class verbatim. Under disable_torch_compile the engine never compiles the encoder, so #836's concern (a recompiled graph rounding differently from the one whose output the prefix cache holds) cannot arise. The auto-merge had taken #836's kwargs-only forward beside #859's forward_batched, which calls forward with the clips positionally; that keep-both is undone. Main's VisionEncoderSubmodule (marks_dynamic_dims, prepared indices) and Thinker changes (cuts_tensor_inputs, split_inputs, mrope_position_ids, image_span) are kept; #859 did not change those. Imports: both cp_embed_accumulate and mrope_position_ids; the audio helpers only #836's prepare_inputs used are dropped. Tests following from it (test/modular/test_qwen3_omni_prefix_cache.py): - the test that #836's prepared chunk tensors embed a clip as the encoder does alone is deleted with that code; - the compile test now asserts the audio encoder stays eager while its output is cached; - the CUDA no-recompile test keeps its vision case only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…igin/main 73c6c17) into merge/860-main Brings main's mstar-project#892 Parakeet / Nemotron ASR, mstar-project#917, mstar-project#897 Ulysses as one captured all-to-all (prefer_all_gather and the all-gather exchange gone, Cosmos3 captures every tier under TP x SP), mstar-project#915, mstar-project#836 Qwen3-Omni Thinker prefix caching, mstar-project#909 and mstar-project#900, with mstar-project#858's and mstar-project#859's resolutions. No conflicts. Files both sides touched (docs/clients.rst, docs/installation.rst, docs/models.rst, sequence_parallel.py, cosmos3/submodules.py, test_accelerator_graph_capture.py, test_flux2_klein_model.py): each merged change equals mstar-project#859's own delta since 3166401, so nothing kept both sides. mstar-project#860's my_rows / gather_rows (Wan2.2's CFG split) sit beside main's new ulysses_attention and use CommGroup.all_gather, which mstar-project#897 left unchanged; nothing in mstar-project#860 passed prefer_all_gather. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stacked under #376 (LTX-2.5 TP/SP), which is rebased on this and uses it.
What changes
CommGroup.all_to_all(mstar/distributed/communication.py) sends one packed buffer withall_to_all_single, instead of a list of contiguous pieces whose received counterparts it then concatenated. Uneven splits pass split sizes; negative dims are normalized first, asall_gatherdoes. The collective stays behindtorch.compiler.disable, so a compiled caller breaks around it and the packing and unpacking compile as their own frame. Tracing it as a functional collective (_functional_collectives.all_to_all_single) keeps one graph and is bit-exact in isolation, but served Cosmos3 SP2 then hit an illegal memory access in the batched capture's warm-up (and in review, the plain collective without the disable, or acustom_opwrapper, gave wrong batched images), so the disable stays.ulysses_attentiontakesseq_dim/head_dim(default0, 1, the[S, H, D]layout it always assumed), so batched layouts such as LTX's[B, S, H, D]use it too. It sends q, k and v in one all-to-all:CommGroup.all_to_alltakes several tensors that differ only in the scattered extent, writes rank r's piece of each one after another into rank r's block of the send buffer (one copy per tensor), and unpacks each on its own, so they come back contiguous and the copies are those of separate exchanges. That is two exchanges per attention instead of four, which matters because at small sizes each one is latency-bound (below). Different query and key/value head counts work, as long as each divides by the group size, as Ulysses already requires.prefer_all_gatherand_ulysses_attention_via_all_gatherare removed. They existed because the all-to-all was believed not to replay from a CUDA graph, so captured paths used an all-gather that moves P× the bytes. The all-to-all does replay (below). Cosmos3's captured denoise drops the flag and its plumbing, and the SP config comments say the capture includes the all-to-all.cosmos3_nano_tp2_sp2on 4× H100 (with [Distributed][Bug fix] Make the members of a gloo subgroup agree on its name, so a TP x SP mesh starts #705, without which TP×SP doesn't start) captures 192×320 and 480×832, and its images match SP2's at 38 and 41 dB PSNR (run in review).This is the exchange #376 wrote for LTX as its own
sequence_to_heads/heads_to_sequence; review there asked for one all-to-all rather than two. #376 now deletes its copy and callsulysses_attention.Evidence that the all-to-all captures
On 2× H200, torch 2.12.1+cu132, NCCL 2.29.7:
dist.all_to_all(list) andall_to_all_singlecapture intorch.cuda.graphand replay bit-exact, including with new inputs copied into the static buffers.ulysses_attentionundertorch.compile(mode="max-autotune-no-cudagraphs"), captured: bit-exact against the compiled eager call and against one rank attending the whole sequence, for even and uneven splits, before and after new inputs.Why the earlier conclusion was reached is not known; an older NCCL/torch, or autotuning during capture (which its docstring also mentions), are guesses.
Performance
Cosmos3-Nano,
configs/cosmos3_nano_sp2.yaml, 2× H200, SHM arena,COSMOS3_GEN_CAPTURE_RES=192x320,480x832,720x1280 COSMOS3_GEN_CAPTURE_BS=1,4,8, so 1280×720 (overgraph_max_latent_area) runs eager on both builds.Single request, 50 steps, median of 8 after 2 warmups per server; base and this PR alternated, three server boots each, the mean of the three medians:
That run was before q, k and v shared an exchange. Sharing it, against the version that sent them separately, alternated, three boots each:
Images byte-identical across the six boots, and 600 more requests (single and 8 at a time) without a failure. Against the base from the first run that is about −7%, −12% and −19% (across runs, so indicative). That run's version laid q, k and v interleaved by head and packed them twice; the head (6eb3f3c) packs once and returns each contiguous, and measured level with it on Cosmos3 (320×192 0.552 vs 0.544 s, 832×480 0.872 vs 0.866 s, 1280×720 2.760 vs 2.764 s, two boots each, images identical). On LTX-2.5 SP2 the interleaved version was ~2% slower than separate exchanges; the head is level with them: 19.52 / 21.72 / 22.30 videos/min at c=1/2/4 against 19.55 / 21.79 / 22.24, two-stage 5.78 / 6.31 against 5.77 / 6.35, video and audio byte-identical.
Where the time goes, from an nsys trace of the captured step (320×192, per request): the exchange's point-to-point
ncclDevKernel_SendRecvtook a median 13.9 µs a call against 8.4 µs for the base'sAllGather_RING_LL, over ~7,200 calls, about 40 ms; GEMM, attention and fused-kernel time were unchanged, and both builds ran the same 50,419 kernels. RaisingNCCL_P2P_LL_THRESHOLDto 1 MiB made it 2–3% slower, so the fix is fewer calls. Separately, ~240 ms of the ~650 ms request is the GPU idle between the 50 denoising steps (host work):MSTAR_TP_ASYNC_SCHED=1cut 9% at 320×192 and 6% at 832×480 with identical images, which is outside this PR.In the first table, the 832×480 gain is where the base captured the all-gather. 1280×720 runs eager, where the base already used the list all-to-all, so its gain is from sending one packed buffer instead. On H100 the captured sizes got slower; that is not reproduced on H200, and its cause is open: the exchange alone in a CUDA graph is faster than the base's (3.1 against 5.1 ms per forward at 320×192, measured in review), and on H200 the extra graph breaks cost nothing measurable in a captured replay (100 against 104 µs with and without).
From an earlier A/B on the same box (two boots each): t2i throughput at 832×480 c=1 1.07 against 0.98 req/s (+9.5%), at c=4 / c=8 −0.2% to +2.8%; t2v 189 frames at 320×192 / 832×480 / 1280×720 within 0.6%.
LTX-2.5 SP2 (
configs/ltx2_5_sp2.yaml, 2× H200), #376 rebased on this, its own exchange vsulysses_attention, alternated with two servers each: 19.6 / 21.9 / 22.3 videos/min at c = 1 / 2 / 4 on both (within ±1%), and the 544×960 video, the two-stage 1088×1920 video and both audio tracks byte-identical across all four servers.Tests
ruff check .: clean.python -m pytest test/modular test/distributed test/chatterbox test/kokoro test/omnivoice test/sampling_test mstar/model/*/tests benchmark/worker_phases,OMP_NUM_THREADS=2, on a 2× H200 box so GPU-guarded tests ran) at 26955cf: 7218 passed, 385 skipped, 21 failed;main(0b1a723) gives 7206 passed, 385 skipped, 21 failed, and the 21 failing tests are the same set (eval-compare job control, SHM shutdown cleanup, and a few capture tests).test/modular/test_waypoint_gpu.pywas run separately (14 passed on both) because running it from several suites at once on one GPU hangs, onmaintoo. At the head (538cbd7),test_sequence_parallel.pyand the Cosmos3 tests: 218 passed, 28 skipped.test/modular/test_sequence_parallel.py(gloo, CPU):CommGroup.all_to_allagainst the layout it promises on 2 and 3 ranks with even and uneven splits, for[S, H, D]and[B, S, H, D], with positive and negative dims;ulysses_attentionagainst one rank attending the whole sequence, with equal and with fewer key/value heads than query heads.CommGroup.all_to_allof several tensors at once against one at a time. At the head (6eb3f3c), with the Cosmos3 tests: 226 passed, 28 skipped.Not done here
unspecified launch failure(after 4 and 9 traced requests) where the base ran ten; 1,000 requests without the profiler on this branch have not failed. Whether it is the profiler with point-to-point NCCL inside graphs or a rarer bug is open.cosmos3_nano_sp4,cosmos3_super_tp2_sp2) were not run;cosmos3_nano_tp2_sp2was, in review (above).attend_shards(the log-sum-exp merge for a replicated query over sharded keys) stays in LTX-2.5: tensor- and sequence-parallel DiT and the two-stage distilled recipe #376.Developed with AI assistance (Claude Code).
🤖 Generated with Claude Code