Repository navigation
[ROCm][Qwen4Exp] Remove the Qwen3.8-Flash-Next TEP8 MXFP4 emulation guard - #60816
Conversation
|
@simondanielsson: could you please review this? 🙏 Pinging also @eppaneamd for visibility. |
maeehart
left a comment
There was a problem hiding this comment.
Looks good to me. In the pinned AITER v0.1.24.post1, the CK-Tile stage 1 already doubles the pad for the fused gate/up weight (n_pad_zeros = intermediate_pad // 64 * 64 * (2 if use_g1u1 else 1)), so the extra doubling at MoE tp_size == 1 masked live rows. The paired GSM8K results are convincing, with 596 to 648 problems fixed against 5 to 12 broken, and TP2, which this PR does not change, stays within noise. Removing _requires_qwen38_tep8_emulation is also justified. Native TEP8 matches the emulation accuracy (0.9666 against 0.9674) at 1.9x to 2.6x the output throughput, and the guard did not fire for the checkpoint card's recipe with fused shared experts.
I have one request. The old comment linked the AITER lines that this rounding follows. Can the aiter_moe_pads docstring point to the AITER code that does the doubling, and say whether the FlyDSL stage 1, which takes inter_dim_pad unchanged, expects the same undoubled value? That makes the next AITER bump easy to check. @tjtanaa, can you take the final look?
Thanks for the review 🙏 . I pushed c41620b, which extends the |
simondanielsson
left a comment
There was a problem hiding this comment.
Thanks for the fix!
Suggestion: can we add a regression test for this? I.e. a test that checks that no weights are being overriden after calling aiter's moe op. The test should fail without the fix
simondanielsson
left a comment
There was a problem hiding this comment.
CC claims this is fixed by #55104 already, can you please check?
…uard _requires_qwen38_tep8_emulation forces OCP MX emulation for Flash Next's routed experts (E=512, H=2560, N=640) at EP8 with 64 local experts on gfx950, because AITER padded N to 768 and the native kernel lost accuracy. Since vllm-project#55104 the W4A4 intermediate is aligned to 128, so N stays at 640 with no pad and the native kernel is accurate: GSM8K 0.9651 native against 0.9629 emulation, at 1.9x to 2.3x the output throughput. Drop the guard, and replace its two tests with one that checks the TEP8 shape selects AITER_MXFP4_MXFP4 under both auto and aiter. Signed-off-by: Mikko Tukiainen <Mikko.Tukiainen@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
c41620b to
836a80c
Compare
Thanks for the review! You were right about the accuracy. #55104 covers most of the original scope of this PR. What it left is |
simondanielsson
left a comment
There was a problem hiding this comment.
Thanks for the simplification!
Suggestion: Can we add full niah and aime25 results to check that the non-emulated kernel indeed is numerically accurate?
Suggestion: can we check if we can add a test for this? I.e. verify that the qwen38 tp8ep moe shapes now match a reference implementation
|
✅ @mjkvaak-amd, CI is now available for this PR.
|
|
/ci run |
simondanielsson
left a comment
There was a problem hiding this comment.
Thanks for attaching the results.
Just following up on this, then we should be good to go
Suggestion: can we add a regression test for this? I.e. a test that checks that no weights are being overriden after calling aiter's moe op. The test should fail without the fix
At MoE tp_size == 1, rocm_aiter_fused_experts doubles intermediate_pad for the fused gate/up weight. For W4A4 SiLU, AITER's stage 1 kernels already double it (CK-Tile n_pad_zeros, FlyDSL inter_dim_pad_total), so the pad is doubled twice and live intermediate rows are masked. Since vllm-project#55104 aligns the W4A4 intermediate to 128 this needs a shard whose width is not a multiple of 128 (e.g. 704 stored at 768); Qwen3.8 Flash Next's 640 was the case before it. Skip the doubling for W4A4 SiLU only, factored into aiter_moe_pads. Tests: test_aiter_mxfp4_w4a4_moe_pad_masks_only_padding runs the AITER W4A4 MoE with zero-padded experts once with the pad declared and once without, and requires the same output and unchanged weights; it fails without this change. test_aiter_moe_pads pins the rounding per activation and quant path. Signed-off-by: Mikko Tukiainen <Mikko.Tukiainen@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Added the test. Could you have a look? 🙏 |
|
/ci run |
|
✅ Triggered Buildkite CI #95345 for commit |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #95345. |
|
/ci run |
|
✅ Triggered Buildkite CI #95419 for commit |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #95419. |
|
/ci run |
|
✅ Triggered Buildkite CI #95733 for commit |
|
/ci run |
|
✅ Triggered Buildkite CI #95856 for commit |
…uard (vllm-project#60816) Signed-off-by: Mikko Tukiainen <Mikko.Tukiainen@amd.com>
Overview
Removes
_requires_qwen38_tep8_emulation, which forces OCP MX emulation for Qwen3.8 Flash Next's (amd/Qwen3.8-Flash-Next-Quark-MXFP4) routed experts at TEP8 on gfx950 when the fused shared expert is off. The guard was added because AITER padded the MoE width from 640 to 768 for that shape and the native W4A4 kernel lost accuracy. Since #55104 aligns the W4A4 intermediate to 128, Flash Next's width stays at 640 with no pad, and the native kernel is as accurate as emulation and about twice as fast.Existing work:
nightly-43b4aaea).tp_size == 1. With [ROCm][MXFP4] Align the AITER W4A4 MoE intermediate size to 128 #55104 merged, that is a no-op for Flash Next, so it is dropped here.Claims
VLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=0, the only configuration where the guard fires, the native AITER W4A4 kernel matches emulation's accuracy (GSM8K 0.9651 vs 0.9629) at 2.3x the output throughput at 8k/1k concurrency 4 (362 vs 158 tok/s) and 1.9x at concurrency 32 (1399 vs 741).E=512, H=2560, N=640at EP8 on gfx950.Validation
Unit tests (MI355X, gfx950,
nightly-5555a2d8with this PR): 5 passed.test_qwen38_tep8_selects_native_aiterreplaces the guard's two tests: the TEP8 shape now selectsAITER_MXFP4_MXFP4under bothautoandaiter.Accuracy, GSM8K 5-shot, all 1319 questions, MI355X, the checkpoint card's recipe,
vllm/vllm-openai-rocm:nightly-5555a2d8vs the same image with this PR, one run per cell. Exact match, higher is better:VLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=0--enable-expert-parallel)Flexible-extract; strict-match for the first row is 0.9636 → 0.9659. Standard error is about 0.005.
Throughput, TEP8 with
VLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=0, emulation (stock) vs native (this PR), random dataset, one run per cell. Output tok/s is higher-is-better, mean TPOT lower-is-better:Reproduce:
Details
Nto 768 for this shape. Since [ROCm][MXFP4] Align the AITER W4A4 MoE intermediate size to 128 #55104 the W4A4 intermediate is aligned to 128, so 640 is stored as is, with no pad. The native kernel at 640 is accurate (first accuracy row), so CK-Tile'sinter_dim % 256scale issue does not show up for this shape.num_local_experts == 64. The fused shared expert adds one local expert per rank (ExpertMapManager), so with the card's recipe the count is 65 and the native kernel already ran; onlyVLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=0hit the guard.Limitations and CI gaps.
tp_size == 1. With [ROCm][MXFP4] Align the AITER W4A4 MoE intermediate size to 128 #55104, Flash Next's pad there is 0, so that part did nothing in these runs; the guard removal is the same code.Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.
Co-authored-by: Cursor cursoragent@cursor.com