[AMD] kimik2.5-fp4-mi355x-vllm-disagg: split P/D by concurrency (1D low, 2D high) / 优化 kimik2.5-fp4-mi355x-vllm-disagg：按并发拆分 P/D 拓扑（低并发 1D、高并发 2D） - #2301
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
c4d6ebf to
238cb25
Compare
|
Claude finished @hongxiayang's task in 3m 56s —— View job PR Review
LGTM - no blocking issues found Verified details:
Minor observations (not blockers): conc 512 is dropped from the 8k/1k sweep (the new lists top out at 256 — documented in the changelog entry), and |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR touches a disaggregated-serving benchmark recipe with a confirmed accuracy-risk bug (inline comment): TP4 workers now run with AITER RMSNorm still enabled, contradicting the precedent set by the sibling single-node recipe for the same model/hardware. Given a real finding here, a human should review before merge.
Extended reasoning...
Overview
The PR modifies the kimik2.5-fp4-mi355x-vllm-disagg recipe: bumps the vLLM ROCm image, switches all prefill/decode workers from TP8 to TP4, splits the 8k/1k topology across two P/D layouts by concurrency, re-pins the vLLM router image, and adds two excluded SLURM nodes. Changes span configs/amd-master.yaml, benchmarks/multi_node/amd_utils/models_vllm.yaml, job.slurm, submit.sh, and perf-changelog.yaml.
Security risks
None — this is an internal benchmark-configuration change (image tags, CLI flags, env vars, SLURM node exclusions). No auth, crypto, or permission-sensitive code is touched.
Level of scrutiny
This is a benchmarking recipe, not production-serving code, so the bar is lower than for user-facing systems. However, the inline finding is a genuine numerical-accuracy regression: the sibling single-node script (kimik2.5_fp4_mi355x.sh) explicitly disables VLLM_ROCM_USE_AITER_RMSNORM for TP<8 due to known accuracy issues, and I confirmed that guard exists in that file. This PR moves the disagg recipe's workers to TP4 without adding the equivalent guard in models_vllm.yaml, so the multi-node sweep now silently runs a numerically-degraded config that only a throughput-focused sweep would never catch. That's a real, actionable finding, not a nit.
Other factors
The three "ruled out" candidate issues (all2all-backend without EP, and two rephrasings of the same AITER RMSNorm concern) were already covered by the confirmed inline finding, so no separate note is needed there. Given the confirmed accuracy bug, I'm deferring rather than approving.
| prefill_flags: "--tensor-parallel-size 8 --no-enable-prefix-caching --block-size 1 --gpu-memory-utilization 0.90 --max-model-len 32768 --mm-encoder-tp-mode data --kv-cache-dtype fp8 --max-num-seqs 256 --max-num-batched-tokens 32768" | ||
| decode_flags: "--tensor-parallel-size 8 --all2all-backend mori_low_latency --no-enable-prefix-caching --block-size 1 --gpu-memory-utilization 0.90 --max-model-len 32768 --mm-encoder-tp-mode data --kv-cache-dtype fp8 --max-num-seqs 256 --max-num-batched-tokens 32768" | ||
| env: "VLLM_USE_V1=1 VLLM_ROCM_USE_AITER=1 VLLM_ROCM_USE_AITER_RMSNORM=1 VLLM_ROCM_QUICK_REDUCE_QUANTIZATION=INT4 HSA_NO_SCRATCH_RECLAIM=1 VLLM_ENGINE_READY_TIMEOUT_S=3600" |
There was a problem hiding this comment.
🔴 The disagg recipe now runs Kimi-K2.5-MXFP4 at TP4 (down from TP8) via configs/amd-master.yaml, but benchmarks/multi_node/amd_utils/models_vllm.yaml:28-30 still hardcodes VLLM_ROCM_USE_AITER_RMSNORM=1 unconditionally with no per-TP branching. The sibling single-node recipe (benchmarks/single_node/fixed_seq_len/kimik2.5_fp4_mi355x.sh:51-54) explicitly disables AITER RMSNorm for TP<8 due to known accuracy issues, so this multi-node recipe now silently ships a numerically-degraded config since the fixed-seq-len sweep only measures throughput. Fix: disable VLLM_ROCM_USE_AITER_RMSNORM when TP<8, matching the single-node guard.
Extended reasoning...
What the bug is
configs/amd-master.yaml's kimik2.5-fp4-mi355x-vllm-disagg recipe is changed by this PR from all-TP8 prefill/decode to all-TP4 for both new topologies (1P/1D and 1P/2D). However, benchmarks/multi_node/amd_utils/models_vllm.yaml:28-30 (the Kimi-K2.5-MXFP4 entry) still statically sets env: "... VLLM_ROCM_USE_AITER_RMSNORM=1 ..." with no conditional logic based on tensor-parallel size. That env string is exported verbatim for every worker regardless of TP.
Why this matters — the sibling recipe already flags this exact hazard
benchmarks/single_node/fixed_seq_len/kimik2.5_fp4_mi355x.sh:51-54 serves the identical model (amd/Kimi-K2.5-MXFP4) and explicitly guards against this:
# Disable AITER RMSNorm for TP < 8 due to accuracy issues
if [ "${TP}" -lt 8 ]; then
export VLLM_ROCM_USE_AITER_RMSNORM=0
fiThis is direct, authoritative evidence from AMD/the repo's own single-node recipe that the AITER RMSNorm kernel has a known numerical-accuracy problem at TP<8 for this specific model/hardware combination. The multi-node models_vllm.yaml config has no equivalent branching — it is a flat, static env string.
The code path that triggers it
server_vllm.sh loads the env field from models_vllm.yaml into MODEL_ENVS and its setup_vllm_env() does a simple for env_pair in ${MODEL_ENVS}; do export "$env_pair"; done — there is no TP-conditional logic anywhere in the multi-node vLLM path. The same script sed-rewrites --tensor-parallel-size to PREFILL_TP_SIZE/DECODE_TP_SIZE, which submit.sh computes as PREFILL_NODES*PREFILL_TP/PREFILL_WORKERS. With the new master config (tp: 4 for both prefill and decode, num-worker: 1 for prefill, num-worker: 1 or 2 for decode), every worker in both new topologies genuinely runs at TP4 — e.g. 1*4/1 = 4 for prefill, and 1*4/1 = 4 (1D) or 2*4/2 = 4 (2D) for decode. So this recipe now lands squarely in the TP<8 zone the single-node script deliberately avoids, but with AITER RMSNorm turned on.
Why nothing catches it
The fixed-seq-len scenario is a throughput-only sweep (RUN_EVAL defaults to false, and this PR makes no eval-harness changes). Accuracy is never measured, so a numerically-degraded serving config would still produce green throughput numbers and merge/publish silently.
Step-by-step proof
- Before this PR:
kimik2.5-fp4-mi355x-vllm-disaggran prefill and decode both at TP8 (old master config:tp: 8for both worker rows). At TP8, AITER RMSNorm is fine per the sibling script's own guard (if TP<8 disable; TP8 is not<8, so it stays enabled — the known-good case). - This PR changes the master config: prefill
tp: 4, decodetp: 4in both search-space rows (1P/1D and 1P/2D). submit.shcomputesPREFILL_TP_SIZE = PREFILL_NODES*PREFILL_TP/PREFILL_WORKERS = 1*4/1 = 4,DECODE_TP_SIZE = 1*4/1 = 4or2*4/2 = 4. Every worker really runs at TP4.models_vllm.yaml'sKimi-K2.5-MXFP4.envstill unconditionally setsVLLM_ROCM_USE_AITER_RMSNORM=1;setup_vllm_envexports it as-is, no TP check.- Result: TP4 + AITER RMSNorm enabled — exactly the combination the single-node recipe's comment says causes accuracy issues, newly introduced by this PR's TP8→TP4 switch.
- The fixed-seq-len sweep only measures tok/s/latency, so this ships green with degraded output quality unmeasured.
How to fix
Add the same per-TP guard used in the single-node script — either branch in server_vllm.sh's setup_vllm_env (or equivalent) to unset VLLM_ROCM_USE_AITER_RMSNORM when the resolved TP is <8, or simply drop VLLM_ROCM_USE_AITER_RMSNORM=1 from the Kimi-K2.5-MXFP4 yaml env now that this recipe runs exclusively at TP4. This is a one-line fix and directly mirrors precedent already in the codebase.
Addressing the "is this just a PR-description mismatch" concern
This is not a stale-description nit — the code itself demonstrably runs a config (TP4 + RMSNorm=1) that the repo's own sibling script treats as a documented accuracy hazard, independent of anything the PR description claims. The PR description even states intent to "sync per-worker vLLM serve flags/env with the single-node recipe" — this is the one flag that was supposed to be synced but was missed.
238cb25 to
ddd809a
Compare
…ow, 2D high) Minimal green recipe on current main (same scope as hy/kk-green-sha; NO barrier or eval-harness changes), plus the P/D concurrency split: - Image bump to vllm/vllm-openai-rocm:nightly-2afa3f7e950264bb179d030c23a1ed1f46558fd9 - All-TP4 prefill/decode, EP off (ep:1); fp8 KV + --max-model-len 32768 / --max-num-seqs 256 / --max-num-batched-tokens 32768; drop the PIECEWISE pin - Split 8k/1k topology: conc 1/2/4 on 1P(TP4)/1D(TP4); conc 8..256 on 1P(TP4)/2D(TP4) - Re-pin VLLM_ROUTER_IMAGE to vllm/vllm-router:nightly-20260716-1fbcde7 (old GC'd) - Exclude known-bad nodes mia1-p01-g09,g14 Co-authored-by: Cursor <cursoragent@cursor.com>
ddd809a to
6bbf17e
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29948517533 |
|
vllm recipe update: vllm-project/recipes#650 |
|
/reuse-sweep-run |
hongxiayang
changed the title
|
/reuse-sweep-run |
seungrokj
left a comment
seungrokj
left a comment
There was a problem hiding this comment.
As a PR reviewer and CODEOWNER, I have reviewed this and have:
- Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
- Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
- Verified that this PR has passed PR validation. Please link to GitHub Action workflow that shows this.
- Verified that this PR passes evals. Please link to GitHub Action workflow that shows this.
- Verified that speculative decoding PRs uses chat templates to align the AL distribution to real world
- For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.
- Verified that the model architecture isn't changed with benchmark hacks like using --hf-overrides to skipping indexer for every x layers on models that don't natively support this. As a general rule, we won't accept optimizations that reduces the number of model architecture FLOPs. Anything that makes that same computation run faster is fair game; FLOPs at lower precisions is fine, given that the config passes private evals. As an general north star princple, we should only use optimizations which is used in production by customers that care about accuracy
- If an company claims that they support vLLM/SGLang as first class LLM inference engines on their hardware, I have verified that the respective vLLM submission made using upstream https://hub.docker.com/u/vllm docker repo, upstream SGLang https://hub.docker.com/u/lmsysorg docker repo. The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet as supported by vLLM/SGLang community maintainers
- If an company claims that they support vLLM/SGLang as first class upstream in-tree LLM inference engines on their hardware, I have have verified that the respective vLLM/SGLang submission has been made before additional frameworks (TRT-LLM, ATOM, etc.). The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet.
- Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or the SGLang cookbook:
- I linked the corresponding upstream PR in the vLLM recipe repo or SGLang repo and verified that it is MERGED before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below.
- Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at docs/waiver/
<PR_NUMBER>.md— named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section. - If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.
Additional detail section:
- insert any additional info here
vllm recipe is at https://github.com/vllm-project/recipes/blob/main/models/moonshotai/Kimi-K2.5.yaml
Signed: seungrokj
✅✅✅ Verdict: PASS ✅✅✅✅ Check 0 (CODEOWNER): PASS — Note: the sign-off conversation comment that triggered this gate (id 5052325108) has since been deleted; the identical checklist stands as |
|
@adibarra @Oseltamivir @functionstackx can you plz take a look at this ? |
Keep kimik2.5-fp4-mi355x-vllm-disagg (SemiAnalysisAI#2301) from main and append minimaxm3-fp4-mi355x-vllm-disagg-agentic (SemiAnalysisAI#2326) at the tail. 中文：解决 perf-changelog.yaml 与 main 的冲突，保留 main 上的 kimik2.5 条目并在末尾追加 MiniMax-M3 agentic 条目。 Co-authored-by: Cursor <cursoragent@cursor.com>
4 participants
Description
Retunes the
kimik2.5-fp4-mi355x-vllm-disaggdisaggregated recipe for MI355X on top of currentmain. The recipe deltas mirror the previously-green run (hy/kk-green-sha, no barrier or eval-harness changes); the new idea is to split the 8k/1k P/D topology by concurrency so each concurrency band runs on the layout that is stable there.Recipe (
configs/amd-master.yaml,benchmarks/multi_node/amd_utils/models_vllm.yaml)vllm/vllm-openai-rocm:nightly-2afa3f7e950264bb179d030c23a1ed1f46558fd9.ep:1; a single-node TP8 sweep showed EP 14–27% slower than dense).--kv-cache-dtype fp8,--max-model-len 32768,--max-num-seqs 256,--max-num-batched-tokens 32768;VLLM_ROCM_QUICK_REDUCE_QUANTIZATION=INT4,HSA_NO_SCRATCH_RECLAIM=1, AITER defaults; drop thecudagraph_mode: PIECEWISEpin (use vLLM's defaultFULL_AND_PIECEWISE).1/2/4on1P(TP4)/1D(TP4)(2 nodes); conc8/16/32/64/128/256/512on1P(TP4)/2D(TP4)(3 nodes). A single decode engine is unstable in the high-concurrency tail for this recipe; running the tail on 2D spreads decode load/KV across two engines.Infra (
benchmarks/multi_node/amd_utils/job.slurm,submit.sh)VLLM_ROUTER_IMAGEtovllm/vllm-router:nightly-20260716-1fbcde7(the previousnightly-20260629-e667ebbwas garbage-collected from Docker Hub).mia1-p01-g09,g14from the disagg node pool.Note on AITER RMSNorm (re: review comment):
VLLM_ROCM_USE_AITER_RMSNORM=1is intentionally kept at TP4. The single-node recipe disables it for TP<8 for a historical accuracy issue (v0.16→0.18 era), but a local TP4 GSM8K A/B on this pinned image showed identical accuracy with RMSNorm on vs off (0.9697 vs 0.9697), so the perf path is kept — same rationale as the green run. Happy to add theTP<8 → RMSNorm offguard if reviewers prefer strict parity with single-node.Results
Full sweep is green: https://inferencex.semianalysis.com/inference?unofficialRun=29948517533
Test plan
full-sweep-fail-fastgreen forkimik2.5-fp4-mi355x-vllm-disagg(run 29948517533).中文说明
在当前
main基础上，为 MI355X 重新调优分离式(disagg)配置kimik2.5-fp4-mi355x-vllm-disagg。配方改动与此前已通过(green)的运行(hy/kk-green-sha，不含 barrier 与 eval 相关改动)保持一致；本 PR 的新思路是按并发(concurrency)拆分 8k/1k 的 P/D 拓扑，让每个并发区间跑在其稳定的布局上。配方改动(
configs/amd-master.yaml、benchmarks/multi_node/amd_utils/models_vllm.yaml)vllm/vllm-openai-rocm:nightly-2afa3f7e950264bb179d030c23a1ed1f46558fd9。ep:1；单节点 TP8 实测显示 EP 相比 dense 慢 14%~27%）。--kv-cache-dtype fp8、--max-model-len 32768、--max-num-seqs 256、--max-num-batched-tokens 32768；VLLM_ROCM_QUICK_REDUCE_QUANTIZATION=INT4、HSA_NO_SCRATCH_RECLAIM=1、AITER 默认值；移除cudagraph_mode: PIECEWISE固定值（改用 vLLM 默认的FULL_AND_PIECEWISE）。1/2/4跑1P(TP4)/1D(TP4)（2 节点）；并发8/16/32/64/128/256/512跑1P(TP4)/2D(TP4)（3 节点）。该配方下单个 decode 引擎在高并发尾部不稳定，将尾部放到 2D 可将 decode 负载/KV 分摊到两个引擎上。基础设施(
benchmarks/multi_node/amd_utils/job.slurm、submit.sh)VLLM_ROUTER_IMAGE至vllm/vllm-router:nightly-20260716-1fbcde7（此前的nightly-20260629-e667ebb已被 Docker Hub 回收）。mia1-p01-g09,g14。关于 AITER RMSNorm（回应评审意见）： 在 TP4 下有意保留
VLLM_ROCM_USE_AITER_RMSNORM=1。单节点配方在 TP<8 时会关闭它（针对 v0.16→0.18 时期的历史精度问题），但在本固定镜像上的本地 TP4 GSM8K A/B 显示开/关精度一致（0.9697 vs 0.9697），因此保留该性能路径——与 green 运行的判断一致。如评审倾向与单节点严格对齐，可加上TP<8 → 关闭 RMSNorm的判断。结果
完整 sweep 已通过(green)：https://inferencex.semianalysis.com/inference?unofficialRun=29948517533
测试计划
kimik2.5-fp4-mi355x-vllm-disagg的full-sweep-fail-fast通过（run 29948517533）。