Repository navigation
fix(moe): pass an explicit output size to the EP all-to-all ops - #4086
Merged
Merged
Conversation
garrett361
force-pushed
the
fix/ep-a2a-static-size
branch
from
October 14, 2026 14:39
c6b3073 to
63cf029
Compare
garrett361
marked this pull request as ready for review
October 14, 2026 16:33
garrett361
marked this pull request as draft
October 14, 2026 18:53
The all_to_all_single and mxfp8_all_to_all fakes declared a data-dependent output length, which Dynamo only captures with fullgraph=True. With the default fullgraph=False every MoE block under EP graph-broke inside the activation checkpoint and ran eagerly. The dispatcher now reads the received-token count from the CPU splits (a captured scalar) and the combine reuses the pre-dispatch routed length, so the fakes have static output shapes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…sis commands Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e the proxy claim Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…4068 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eport Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
garrett361
force-pushed
the
fix/ep-a2a-static-size
branch
from
October 22, 2026 14:19
938ad25 to
b0b231f
Compare
garrett361
marked this pull request as ready for review
October 22, 2026 14:29
Member
|
lgtm! |
garrett361
added a commit
that referenced
this pull request
Oct 22, 2026
Ulysses context parallelism published the full-sequence `cu_seqlens` and `max_seqlen` through a module global, `ULYSSES_PARAMS`, which attention read at call time. Compiled blocks guard on that global, so a micro batch with a different document count recompiles every rank. This PR drops the global: attention uses the `cu_seqlens` and `max_seqlen` it is already passed, which the models build from the pre-shard `seq_lens` and so cover the full sequence each rank holds after the all-to-all. - `ULYSSES_PARAMS`, `update_ulysses_params` and `setup_cp_attention_params` are gone, so `setup_cp_params` only shards inputs. - Both trainers check once per micro batch, on the CPU `seq_lens`, that they sum to the full sequence length. This also covers the Qwen3.5 VLM path that defers sharding to the model, and never syncs with the GPU (main's check made three host syncs per micro batch). - GPT-OSS's Ulysses attention gets the same change. - Rebased onto main after #4134 removed ring CP, so this is now Ulysses only; the ring half of the original PR is gone with it. The recompile shows only where blocks compile. On main, EP MoE blocks run eagerly under the default `fullgraph = false` (fixed by #4086), so on the MoE workload below this PR alone is speed-neutral. On top of #4086 it removes the recompile and its slow step: | 1 x 8 B300, GLM-4.5-Air 8 layers, ep 8, cp 2, real data | median (min-max) s | recompiles per rank | `ULYSSES_PARAMS` guard failures | | --- | --- | --- | --- | | main | 2.246 (2.183-2.344) | 1 | 0 | | this PR | 2.231 (2.191-2.339) | 1 | 0 | | #4086 | 1.718 (1.672-8.126) | 3 | 2 | | #4086 + this PR | 1.702 (1.664-1.819) | 1 | 0 | Measured at `b57635d84`; the later commit only moves the `seq_lens` check to the CPU in the trainers. Loss agrees across arms within 0.003 over steps 1-9 and within the repeat spread after that. Baseline: [`fea1267ef`](fea1267). Harness, configs and results: [`REPORT.md` at f3a57c5](https://github.com/PrimeIntellect-ai/prime-rl/blob/f3a57c55f5b7922df3a6d0f2d30c8497df6603ea/benchmarks/scripts/cp-ulysses-params-b300/REPORT.md), removed in the last commit. The ring-era results are in the earlier report at [`47070d3d2`](https://github.com/PrimeIntellect-ai/prime-rl/blob/47070d3d281452d748cd64cfa562f05bb9c5eddd/benchmarks/scripts/cp-ring-params/REPORT.md). GPU unit tests on the same node pass and match main: `test_moe.py`, `test_checkpointing.py`, the Qwen3.5 CP setup test and the 2-rank GPT-OSS CP attention test. Unrelated bugs found along the way, not fixed here: - GPT-OSS calls FA4 directly because the FA4 custom op has no sink argument, so it graph-breaks and cannot use fullgraph. - The color-codeword RL env fails in the orchestrator with `Inline image count does not match expanded placeholder runs`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
garrett361
added a commit
that referenced
this pull request
Oct 22, 2026
#4177 removed model.moe_router_dtype, which broke base.toml's fp32 pin and the bf16_router feature row. The base now keeps the ModelConfig defaults, and a fullgraph=True row on qwen3_moe's cp2_ep4 layout turns any graph break in the EP all-to-all (#4086) or Ulysses CP (#4068) paths into a hard failure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
garrett361
added a commit
that referenced
this pull request
Oct 28, 2026
#4177 removed model.moe_router_dtype, which broke base.toml's fp32 pin and the bf16_router feature row. The base now keeps the ModelConfig defaults, and a fullgraph=True row on qwen3_moe's cp2_ep4 layout turns any graph break in the EP all-to-all (#4086) or Ulysses CP (#4068) paths into a hard failure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
garrett361
added a commit
that referenced
this pull request
Nov 4, 2026
#4177 removed model.moe_router_dtype, which broke base.toml's fp32 pin and the bf16_router feature row. The base now keeps the ModelConfig defaults, and a fullgraph=True row on qwen3_moe's cp2_ep4 layout turns any graph break in the EP all-to-all (#4086) or Ulysses CP (#4068) paths into a hard failure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR passes the EP all-to-all's output length into the op, instead of creating a dynamic size inside the op's fake, so MoE layers compile under the default
fullgraph = false. On main, that fake breaks the graph inside the activation checkpoint and every MoE layer runs eagerly; with this PR they matchfullgraph = truespeed, 1.3x to 2.0x faster on B300.int(output_splits.sum())from CPU splits;_mxfp8_all_to_allgets the same change.fullgraph = truealready compiled these layers because it implicitly turns oncapture_dynamic_output_shape_ops.ULYSSES_PARAMSguard (one 8.1 s step); fix(cp): pass cp attention params as forward args #4068 removes it.This PR has 0 graph breaks in every arm, against 1 on main, and loss agrees within same-code repeat noise. Main with
fullgraph = trueruns the MXFP8 arm in 0.716 s. Baselinefea1267ef; harness and full results inREPORT.mdat 82984c8a8, removed in the last commit.Qwen3-30B-A3B layer 2 forward, rank 0, on H200 (an earlier version of this PR with the same fix). Eager elementwise, copy and index kernels (orange) become a few fused Inductor kernels (green): 16.3 ms to 11.0 ms.
🤖 Generated with Claude Code