Repository navigation
[Model] MAGI-2: head-sharded expert parallelism with an independent EP degree - #8511
yeahdongcn wants to merge 6 commits into
Conversation
5763f2c to
7ac5b76
Compare
015ce08 to
5fe48b5
Compare
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/diffusion/index.md, docs/design/module/diffusion/parallelism.md. Module owners: @princepride @wtomin @david6666666 Routing: @princepride via module of the changed files, module named in the PR description, CODEOWNERS; @wtomin via module of the changed files, module named in the PR description, CODEOWNERS; @david6666666 via module of the changed files, module named in the PR description @yeahdongcn, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot routing recordAssigned Strict on zcode (GLM-5.3-Flash) under experiment |
Omni ReviewBot: three questions on the performance claim@yeahdongcn this PR reads as a performance or value claim:
Before the full evidence checklist, three short questions:
When you answer, the evidence that settles it is: base and head SHA, hardware, model, workload, warm-up and repeat count, mean or percentiles with their spread, and a correctness/quality-equivalence signal; an end-to-end claim also needs stage attribution. |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: c6f0781c-cfb6-4e11-8f41-e4eb08e17628) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: af45b65f-dedc-4f79-a021-72dd13a9b1da) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 40085e0e-1c75-4057-9d85-56ee12485783) — check |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
1 actionable finding(s).
CI at
5fe48b548484(2026-10-10T03:37:58.612346+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/zcode/GLM-5.3-Flashcould not complete this review, so it was produced by the fallback armdirect/cursor/auto. It is excluded from the routing experiment.
Full review analysis
PR description
MAGI-2 can now declare head-sharded expert parallelism and set expert_parallel_size to a divisor of the sequence-parallel degree, instead of always using one expert group per SP group. The diffusion parallel state builds those EP groups as contiguous SP subgroups, and the MAGI-2 MoE dispatches on that subgroup while the transformer passes host token counts in EP rank order. On MUSA, when several of those subgroups tile one SP group and each subgroup has the same token total, the same head shard is exchanged over the whole SP group; other platforms, and any call whose EP groups have different token totals, keep the exchange inside each EP group. With expert parallelism left off, group construction and the MoE exchange stay on the previous path.
Change flow
flowchart LR
cli["[CHANGED] expert_parallel_size on stage config and T2V CLI"]:::changed
groups["[CHANGED] head-EP groups as SP subgroups"]:::groups
layout["[NEW] HeadParallelLayout packed head exchange"]:::new
moe["[CHANGED] MAGI-2 MoE dispatch and combine"]:::changed
out["[EXISTING] per-token MoE head rows"]:::existing
cli --> groups --> moe
groups --> layout --> moe --> out
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
class groups changed
See inline comments below.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
5fe48b5 to
4fbea97
Compare
|
MAGI-2's MoE has 12 heads, sharded over the EP ranks. With EP equal to SP=8 they are padded to 16, so six ranks route two heads over all of the forward's tokens and two ranks hold only padding; with EP4 every rank routes three heads over the tokens of its four-rank group, which gives the busiest rank a quarter fewer (head, token) rows. The MoE grouped GEMMs are the largest item in a profiled EP4 step, about 300 of 740 ms. Against SP4xCFG2 the profiled steps attribute the gap to the 4-way Ulysses all-to-all (about 15 ms more per step than 8-way) and to the two CFG branches carrying different token counts (about 10 ms of extra compute on half the ranks). What it buys is per-step latency for one MAGI-2 T2VA request on 8x MTT S5000 (272p, 125 frames, 100 steps): SP8xCFG1 with EP4 runs at 717.5 ms per step, against 864.6 at SP8xCFG1 without EP and 774.4 at SP4xCFG2, the faster layout without EP. The cost is memory: each rank holds the experts of three MoE heads instead of two at EP8. With EP off nothing changes, and the output is byte-identical. Both deltas blend two mechanisms. With EP on, the transformer passes the MoE its token counts from the host, so the EP arms skip the per-layer size all-gather and host reads that the EP-off arms still do. In the same session, passing the counts with EP off at SP4xCFG2 takes 775.8 to 733.6 ms (this PR passes them only when EP is on); against that, EP4 is 2.2% faster (717.5 vs 733.6). The head layout alone, with the counts passed in both arms: EP8 takes 12.6% longer per step than EP4 (two runs each), with byte-identical video and audio. Commit 4 has its own A/B in the body (688.80 vs 703.14 / 700.34 ms). |
|
@yeahdongcn GitHub currently reports merge conflicts for head |
No problem. Let me do a rebase. Thanks! |
Signed-off-by: Xiaodong Ye <xiaodong.ye@mthreads.com>
Signed-off-by: Xiaodong Ye <xiaodong.ye@mthreads.com>
MAGI-2 declares the head expert-parallel style, so expert_parallel_size can be set to a divisor of the sequence-parallel degree. The MoE then uses the head-EP subgroup of the SP ranks, and the transformer passes each layer the token counts in EP rank order instead of gathering them. expert_parallel_size is declared in the structured stage config, so the engine's kwargs path accepts it, and is exposed as --expert-parallel-size in the text-to-video example. Head-EP requires tensor_parallel_size=1. Signed-off-by: Xiaodong Ye <xiaodong.ye@mthreads.com>
When expert_parallel_size is smaller than the SP degree, the head-EP groups tile the SP group, and ranks at the same position of every group own the same MoE head shard with the same weights. On MUSA the MoE dispatch and combine then run over the SP group: each rank splits its tokens into contiguous parts, one per EP group, and sends each part to the owners in that group, so the exchange reaches every peer instead of only the ranks of one EP group. The parts are chosen so every owner receives exactly as many tokens as the exchange inside its own EP group gives it. When the EP groups hold different token totals, that would put tokens into batches of another size, so that call keeps the exchange inside each EP group. Only which rows share an owner's batch, and their order, change; routing, both grouped GEMMs and the top-k sum compute every (token, head) row on its own, and the combine returns each row to its original position. HeadParallelLayout gains replica_counts for this layout. With EP equal to SP, and on other platforms, the exchange stays inside the EP group. Signed-off-by: Xiaodong Ye <xiaodong.ye@mthreads.com>
enable_expert_parallel keeps its own description, and expert_parallel_size gets one: the expert-parallel degree for head-sharded models, which divides the sequence-parallel degree, defaults to it and requires enable_expert_parallel. Signed-off-by: Xiaodong Ye <xiaodong.ye@mthreads.com>
4fbea97 to
9418cd5
Compare
|
Reviewed head [P2] Document the new head-EP configuration —
No high-confidence runtime blocker found in the source review. The refreshed affected-test results after the latest rebase remain outstanding; CodeQL and the docs check are still pending at this check. Validation: Python syntax parsing and |
Purpose
MAGI-2 shards its MoE experts along the head axis over the Ulysses ranks, so today the expert-parallel degree always equals the sequence-parallel degree. This PR lets a model declare head-sharded expert parallelism and set
expert_parallel_sizeto a divisor of the SP degree, as proposed in #7085. It consolidates and supersedes #7266 and #7270. Five commits:[Refactor]Share packed head-parallel exchanges for diffusion models: a helper invllm_omni/diffusion/distributed/head_parallel.pyfor the packed head all-to-all, used by MAGI-2'sparallel.py(from [Refactor] Share packed head-parallel exchanges for diffusion models #7266).[Feature]Support model-declared head-sharded expert parallel groups:DiffusionModelMetadata.expert_parallel_style="head"makesparallel_statebuild the EP group as SP subgroups of sizeexpert_parallel_size(from [Feature] Support model-declared head-sharded expert parallel groups #7270).[Model]MAGI-2: enable head-sharded expert parallelism. MAGI-2 declares the head style; its MoE uses the head-EP subgroup, and the transformer passes each layer the token counts in EP rank order instead of gathering them.expert_parallel_sizeis declared in the structured stage config, so the engine's kwargs path accepts it, and is exposed as--expert-parallel-sizein the text-to-video example. Head-EP requirestensor_parallel_size=1.[Perf]MAGI-2: exchange head-EP tokens over the whole SP group on MUSA. When EP is smaller than SP (SP8xCFG1 with EP4), ranks at the same position of every EP group own the same MoE head shard with the same weights. On MUSA the MoE dispatch and combine then run over the SP group: each rank splits its tokens into one contiguous part per EP group and sends each part to the owners in that group, so the all-to-all spans all 8 SP ranks instead of the 4 of one EP group.HeadParallelLayoutgainsreplica_countsfor this layout. The output stays bit-exact: each part goes to an owner that holds the same head shard with the same weights, and each owner receives exactly as many tokens as the exchange inside its own EP group gives it, so every MoE batch keeps its size. Only which rows share an owner's batch, and their order, change, and the MoE computes each (token, head) row independently of the rest of its batch;test_owner_moe_rows_do_not_depend_on_their_batchchecks this on the device at the production shape. When the EP groups hold different token totals, keeping those sizes would move tokens into batches of another size, so that call falls back to the exchange inside each EP group. With EP equal to SP, and on other platforms, the exchange is unchanged.[Misc]Documentexpert_parallel_sizeonDiffusionParallelConfig:enable_expert_parallelandexpert_parallel_sizeeach carry their own docstring. No behaviour change.Without
--enable-expert-parallelnothing changes: at SP4xCFG2 the generated video and audio are byte-identical to the same stack without this PR.Part of the MAGI-2 work split from #7156. MAGI-2's MUSA support is in main since #8498.
Test Plan
python -m pytest -q -o addopts='' \ tests/diffusion/distributed/test_head_parallel.py \ tests/diffusion/distributed/test_head_expert_parallel.py \ tests/diffusion/distributed/test_expert_parallel_size_plumbing.py \ tests/diffusion/distributed/test_expert_parallel_layout.py \ tests/diffusion/distributed/test_expert_parallel.py \ tests/diffusion/models/magi2/test_ep_replicas.py \ tests/diffusion/models/magi2/test_head_ep_activation.py \ tests/diffusion/models/magi2/test_pipeline_magi2.py \ tests/config/test_omni_config.py tests/config/test_config_factory.py \ tests/config/composable_parallel/test_translator.pytest_ep_replicas.pyand the new cases intest_head_parallel.pycover the replicated layout and its split sizes, the per-call fallback, a four-rank Gloo MoE forward over the SP group against the EP-group exchange, and on MUSA the production-shape row invariance and an MCCL MoE forward over the SP group against the EP groups, compared withtorch.equal.Functional run through the shared entrypoint, SP8xCFG1 with EP4:
End to end: MAGI-2 Preview T2VA, the prompt above, 272p (448x256), 125 frames, 100 steps, seed 42,
diffusion_compile_dynamic=False, on 8x MTT S5000. The per-step figures come from an out-of-tree timing harness aroundAsyncOmniwith the per-step boundary from #7274; the metric is the per-stepsampler.diffusetime, maximum over the 8 ranks, mean over steps 2-99.Byte identity of commit 4: Inductor benchmarks reduction configs on every fresh compile, so the output of one code base differs across fresh caches, and
TORCHINDUCTOR_DETERMINISTIC=1does not reach the compiled regions: the MAGI-2 pipeline callstorch.use_deterministic_algorithmsat construction, which resets Inductor's deterministic flag. The comparison therefore ran withTORCHINDUCTOR_DETERMINISTIC=1 TORCHINDUCTOR_FORCE_FILTER_REDUCTION_CONFIGS=1and a freshTORCHINDUCTOR_CACHE_DIRper run, and compares the sha256 of the generated video and audio. GEMM shape padding is still chosen by benchmarking under this setting, so differing hashes alone would not show a change in numerics; identical hashes do show identical output.vLLM Version: 0.28.0 (MUSA image
vllm:v0.28.0-ph1-5.2.0-torch2.11.0.post2-20261001), with a local test-only shim for the vLLM 0.30 and 0.31 names that c1e84ce and 198c210 import; the CPU run on the rebased branch used vLLM 0.31.0. Commit 4's A/B ran with torchada MooreThreads/torchada#120; the fixed-config byte-identity runs and the test suites also had torchada'scpp_extensionpath-signature fix MooreThreads/torchada#121 (host-side build paths only), without which cold-cache CPU Inductor compiles fail on MUSA. Both are merged.vLLM-Omni Commit: 4fbea97 on top of main 198c210. The device results below were taken on c1e84ce (head 015ce08), except the second Test Result bullet, which is at 4fbea97. The changes of 4fbea97 differ from those of 362c219c0 only in
parallel_state.py, where main now annotates the_PPand_SPassignments itself, so this PR no longer wraps them incast. Against c1e84ce the branch also restoresFieldin the pydantic import ofvllm_omni/diffusion/data.py, which main dropped as unused, and commit 5 changes only documentation. Commits 1-3 were tested at 5763f2c, which differs only in one test comment and two unreachable lines in an overridden engine method. Commit 4 was tested in an integration tree with the other pending MAGI-2 changes, where the code of this commit is identical; only thecurrent_omni_platformimport inmodeling_magi2.pycomes from another pending change there.Test Result
test_frontend_log_stats_flag_is_not_an_unowned_stage_argument[True]/[False]andtest_mps_stays_in_runtime_instead_of_engine_arguments, which need an accelerator platform).test_ep[tencent/HunyuanImage-3.0](downloads a config), which fails the same way on main.test_two_rank_musa_exchange,test_real_four_rank_musa_head_ep_reinitialization,test_moe_forward_over_the_sp_group_is_bit_exact_on_musaandtest_owner_moe_rows_do_not_depend_on_their_batch(both token layouts) pass. SP8xCFG1 with EP4 under static compile, run interleaved with 362c219c0 (two runs each): byte-identical video and audio, and the same per-step time within run-to-run noise.tests/config/test_omni_config.py::test_diffusion_stage_payload_keys_roundtripand fail identically on c1e84ce without this PR. With all devices visible, the two head-parallel files pass in full (44 passed), including the real MCCL teststest_two_rank_musa_exchangeandtest_real_four_rank_musa_head_ep_reinitialization.test_expert_parallel_size_plumbing.pycovers that--expert-parallel-sizereaches the parallel config through this kwargs path.test_ep_replicas.py,test_head_parallel.pyandtest_head_ep_activation.py75 passed, 0 skipped, includingtest_owner_moe_rows_do_not_depend_on_their_batch(both token layouts) andtest_moe_forward_over_the_sp_group_is_bit_exact_on_musa. The MAGI-2, head-parallel, FA3-adapter and mHC suites on that tree: 818 passed, 4 failed; the 4 aretest_native_compile*.pywithNameError: name 'device' is not defined, which fail identically on plain c1e84ce in this image (fix(device): register the device factory as the FX device builtin MooreThreads/torchada#124 (merged) fixes theNameError; the distributed test then stops at torch_musa, which does not support FSDP2 on a CPU mesh).TORCHINDUCTOR_DETERMINISTIC=1and a fresh Inductor cache per run: 688.80 ms per step with it vs 703.14 / 700.34 without it (-12.9 ms, -1.8%). The output hashes matched the base runs; byte identity rests on the fixed-config check below, since that setting alone does not fix the reduction configs.diffusion_compile_dynamic=True, the default), SP8xCFG1+EP4: control 761.00 ms; integrated 785.71 ms; integrated without [Perf] MAGI-2: drop the Ulysses send/receive buffer copies #8595 741.57 ms; without the second commit of [Perf] MAGI-2: use FP32 mHC stream contractions on MUSA #8510 807.50 ms; without [Perf] MAGI-2: run the MUSA mHC Sinkhorn iterations in one Triton kernel #8594 759.46 ms. Without [Perf] MAGI-2: run the MUSA mHC Sinkhorn iterations in one Triton kernel #8594 the output is identical to the dynamic control, so every other change is byte-identical under dynamic compile too; with [Perf] MAGI-2: run the MUSA mHC Sinkhorn iterations in one Triton kernel #8594 the output equals the static-compile output (see [Perf] MAGI-2: run the MUSA mHC Sinkhorn iterations in one Triton kernel #8594). Only runs with identical outputs are compared for time: the eager MoE GEMM time follows the routing, which changes with the output bits.test_musa_pipeline.py, 2setup_compiletests intest_native_compile.py), 4 are thetest_native_compile*.pyFXNameErrorfrom torchada'storch.devicereplacement (fix(device): register the device factory as the FX device builtin MooreThreads/torchada#124 (merged) fixes theNameError; the distributed test then stops at torch_musa, which does not support FSDP2 on a CPU mesh), 1 istest_expert_parallel.py::test_ep[tencent/HunyuanImage-3.0](downloads a config), and 8 aretest_omni_config.py::test_diffusion_stage_payload_keys_roundtrip, which fail the same way on c1e84ce.AI assistance: Claude Code drafted the change, the tests and this description and ran the MUSA validation listed above. I reviewed every change in this PR and re-ran the validation listed above.