Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces view_aiter_fused_rms_transposed_fp8_scale in fp8_utils.py to correctly restore logical [M, G] indexing for AITER fused-RMS transposed FP8 scales without copying physical bytes. This utility is integrated into the DeepSeek-V4 model's fused RMSNorm FP8 quantization path when _use_aiter_bpreshuffle_gfx95 is enabled. Comprehensive unit and integration tests have been added to validate the layout contract and correct behavior. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Picking up the pending two-node TP8/DP8/EP8 gate you flagged — I have a MI355X 1P1D disagg repro/harness for exactly these legs. Dispatched this PR head ( Will post the GSM8K numbers when it lands. Nice find on the DeepSeek-V4 fused-RMS producer being the missed case — that explains why a consumer-only attempt on my side (#31617, |
|
✅ Two-node TP8/DP8/EP8 gate — PASS. run 29773160421 on this PR head (
Both jobs |
|
Extra corroboration — same two configs, stock main vs this PR, on the same MI355X 2N 1P1D disagg harness:
On stock main these two dp8ep8 flash legs are the only ones in the thin regressed band (0.911 / 0.914) — they passed nightly #114 by noise, right on the 0.91 gate. Every other DSV4 leg on that nightly sits at 0.917–0.946 with margin (and Kimi non-MTP is 0.948 after #31368). This PR lifts the two affected legs back into the ~0.925 good band with real headroom (+~0.013, matching #31490's measured effect). Net: this is the change that takes those legs from "flickering on the gate" to safely green. |
|
Independent verification on MI355X — confirmed on both the dp8ep8 1P1D and the wide-EP16 2P1D topologies. Overlaid only the two changed files ( dp8ep8 1P1D (image
Matches your TP8/DP8/EP8 numbers (fp8 0.923 / fp4 0.928) — the fix lands the affected legs back in the ~0.925 band with headroom. Wide-EP16 2P1D (4-node, image
The four Flash legs are the only ones that sat in the thin regressed band; on EP16 the two non-MTP Flash legs were actually below the 0.91 gate on a 3-run mean (0.902 / 0.901), and this PR clears all of them. Pro was already green and stays green (no regression). Net: confirmed effective across dp8ep8 and wide-EP16, FP8 and FP4, base and MTP. LGTM from the EP16 side too. |
|
Independent confirmation from a different workload. I saw the same issue in DeepSeek-V4-Flash FP8 RL training on four MI355X nodes. I measured At step 0, I kept the weights, nodes, and recipe the same and only changed the code. The normal run-to-run noise is about 0.0015.
A known-good tree gives 0.04452 on the same setup, so adding this PR brings the result back to the healthy range. |
|
Thanks @michaelzhang-ai and @HaiShaw with new checkpoint weights for DeepSeek V4 Flash released today and published strong benchmark, please consider getting this enhancement fix in. https://huggingface.co/deepseek-ai/DeepSeek-V4-Flash-0731
|
AITER's fused-RMS with transpose_scale=True returns transposed physical bytes behind row-major-looking metadata, so downstream CK materialization permutes the scales. sgl-project#31727 restores the logical view with a zero-copy as_strided. That PR has been open since 2026-07-19 and is not in main or sglang-miles, so carrying it here is the only way this branch gets it. It had been living as a container-side patch script, which meant it disappeared the moment the containers were rebuilt on the new image: this run came back at train_rollout_logprob_abs_diff 0.1696 against 0.0448 on a tree known to be healthy, with SGLANG_OPT_USE_JIT_NORM=false confirmed present in all eight schedulers. A patch that only exists inside a container is not carried by anything -- not git, not the md5 check, not the env readback. Taken verbatim from the PR, tests included, so it drops out cleanly if it lands upstream. Co-authored-by: Zhiyao Jiang <jessicajiang324@gmail.com>
|
@hdt98 please post test results from new tests added. |
|
@amd-bot ci-status |
CI Status for PR #31727Merge verdict: Already merged (as Note The changed paths are covered by tests that ran and passed on this head SHA:
Caveat (author-acknowledged in the PR): the exact two-node TP8/DP8/EP8 MORI validation was not run in CI; only TP4/DP4/EP4-equivalent coverage exists. That topology remains unverified by PR CI. Changed files: Executed CI failure attribution: AMD: 1 executed failure (0 related) · Others (NVIDIA/XPU): 2 executed root-cause failures (0 related). All remaining AMD Executed Failures
Other Executed Failures
Details / what to do before merge (moot — already merged)
Generated by amd-bot using Claude Code CLI |
…ducer tests, M==1 gate) Addresses kkHuang-amd's review on sgl-project#33166: - Consolidate the two identical stride-reinterprets into one producer-neutral helper. Main gained view_aiter_fused_rms_transposed_fp8_scale (sgl-project#31727) after this PR opened; it and this PR's bpreshuffle_fp8_scale_nocopy both do as_strided(scale, scale.shape, (1, scale.shape[0])). Merge into view_transposed_fp8_scale_nocopy(+_tuple); repoint deepseek_v4 (fused-RMS), deepseek_v2 (MoE down), forward_mla (MLA o_proj). - Extract the emit gate (previously the triplicated `_use_aiter_bpreshuffle_gfx95 and shape[0] >= 2`) into emit_transposed_bpreshuffle_scale(m, on_bpreshuffle_gfx95=...), so the M>=2 boundary is centralized and unit-testable. - Add an MI35X producer-level test that invokes both real producers (fused_clamp_act_mul, fused_flatten_fp8_group_quant) with transpose_scale True+nocopy vs False+materialize, asserting bit-identical quantized output, equal scale values, the (1, M) column-major stride, and zero-copy sharing. This is the direct producer-level evidence the CPU stride test could not give. - Pin the M==1 materialize fallback: gate returns False at M==1 (materialize path), and the materialized single-token scale keeps its values and (1,1) stride. Producer test also exercises M==1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or consistency Per review follow-up, consolidate onto main's existing view_aiter_fused_rms_transposed_fp8_scale (sgl-project#31727) rather than a new neutral name; add a matching _tuple variant and repoint the deepseek_v2 (MoE down) and forward_mla (MLA o_proj) producers and the tests to it. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…al-path test, M==1) Addresses kkHuang-amd's review on sgl-project#33165: - Consolidate the inline `torch.as_strided(x_scale, x_scale.shape, (1, x_scale.shape[0]))` in aiter_w8a8_block_fp8_linear onto the shared view_aiter_fused_rms_transposed_fp8_scale helper (main sgl-project#31727), for both the fresh-quant no-copy branch and the pre-existing triton row-major-view branch, so the metadata repair lives in one documented place. - Add an MI35X real-path test (test_fp8_bpreshuffle_dense_linear_mi35x.py): quant-producer equivalence (aiter_per1x128_quant transpose_scale True vs False: identical quantized bytes, equal scale values, (1, M) stride, zero-copy) and end-to-end aiter_w8a8_block_fp8_linear new-path vs old-path (forced via patch) bit-exact GEMM output. Exercises the real quant + CK bpreshuffle GEMM. - Pin the M==1 materialize fallback: CPU test for the single-row materialize values/(1,1) layout, and M==1 covered in the MI35X real-path test. - Update the CPU test to use the shared helper instead of a local copy of the as_strided expression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 participants
Note
Exact two-node TP8/DP8/EP8 MORI validation remains pending; the closest available TP4/DP4/EP4 results are below.
Motivation
Fixes #31490. DeepSeek-V4-Flash FP8/FP4 on gfx950/ROCm 7.2 regressed from the ~0.925 GSM8K band to ~0.912 in the 1P1D DP-attention workload.
Root Cause
DeepSeek-V4's separate fused-RMS FP8 producer calls AITER with
transpose_scale=True. AITER writes CK-ready transposed scale storage but returns contiguous-looking[M, G]metadata. For logical scales[[a,b,c],[d,e,f]], the storage is[a,d,b,e,c,f]; it needs stride(1, M), while contiguous metadata reads it as[[a,d,b],[e,c,f]].The CK materializer added by #29275 trusted that metadata, associating valid scales with the wrong token/group cells when
M > 1. Real-AITER A/B tests show bit-identical FP8 activations, an unchanged BF16 side output, and exact logicalscales after the zero-copy stride repair with the same storage pointer.
M = 1remains exactly unchanged. This tuple is injected only into DeepSeek-V4 attention-side dense GEMMs.My thought is this points toward batch dependence and excludes wholesale activation corruption, but it was my mistake in not covering this case: a scale permutation can still cause a large GEMM error. The ~1–2 GSM8K-point effect is the measured end-to-end result, not a bound.
The #31490's
TP == DPtopology has attention TP size one. A matching TP4/DP4/EP4 trace that I was able to run saw zero attention gathers and CK in all 240 relevant GEMMs, excluding the initially suspected Triton/collective path from this issue.Modifications
[M, G]strides at the DeepSeek-V4 AITER producer with a zero-copy view.The shared DeepSeekV2/GLM producers, generic CK materializer, communicator, and Triton path are unchanged.
Accuracy Tests
DeepSeek-V4-Flash, two MI35x nodes, 1P1D TP4/DP4/EP4 per role, MORI, 8-shot GSM8K (1319 questions):
The stochastic runs show recovery and compatibility with current and future CK #8639.
Speed Tests and Profiling
Controlled candidate/stock; same model, topology, image, warm-ups, and 1024-token input/output workload:
No TPOT or output-throughput regression was observed.
Checklist
pre-commit run --all-filespassed.Review and Merge Process
Exact TP8/DP8/EP8 MORI remains the final integration validation gate before merge.
CI States
Latest PR Test (Base): 🚫 Run #30687653207
Latest PR Test (Extra): ❌ Run #30687653166