[AMD] [AGENTX] Minimax-M3 Perf Tuning - #2671
Conversation
Signed-off-by: Sirra <asirra@amd.com>
|
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 关于重新运行失败任务的文档 |
Signed-off-by: Sirra <asirra@amd.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| export VLLM_ROCM_USE_AITER=1 | ||
| export VLLM_ROCM_USE_AITER_MOE=1 | ||
| export VLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=1 | ||
| # The AITER page-16 sparse-attention path requires exactly one KV head per | ||
| # tensor-parallel rank. MiniMax-M3 has four KV heads, so TP4 uses that fast | ||
| # path while TP2 uses vLLM's supported Triton sparse-attention fallback. | ||
| if [ "$TP" -eq 4 ]; then | ||
| export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1 | ||
| else | ||
| export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=0 | ||
| fi | ||
| export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1 |
There was a problem hiding this comment.
🔴 Removing the TP-conditional guard on VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT and hard-coding it to 1 violates the AITER page-16 fast path's documented requirement of exactly one KV head per TP rank (MiniMax-M3 has 4 KV heads). configs/amd-master.yaml's minimaxm3-fp4-mi355x-vllm-agentic-mtp still has a live { tp: 2, ... } arm, which will now launch with 2 heads/rank under the shuffled layout — a topology the deleted comment explicitly says only supports the Triton fallback (shuffle=0). Restore the if [ "$TP" -eq 4 ] guard, or confirm the new v0.27.1 image no longer needs it and drop the TP2 arm instead.
Extended reasoning...
The bug: The previous version of this script conditioned VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT on the tensor-parallel size, with a comment explaining why:
# The AITER page-16 sparse-attention path requires exactly one KV head per
# tensor-parallel rank. MiniMax-M3 has four KV heads, so TP4 uses that fast
# path while TP2 uses vLLM'\''s supported Triton sparse-attention fallback.
if [ "$TP" -eq 4 ]; then
export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1
else
export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=0
fiThis PR deletes both the comment and the conditional, replacing them with a single unconditional line (benchmarks/single_node/agentic/minimaxm3_fp4_mi355x_mtp.sh:126-129):
export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1Why this matters here specifically: MiniMax-M3 has 4 KV heads. The deleted comment states the AITER page-16 sparse-attention fast path (which this shuffle env var enables) requires exactly 1 KV head per TP rank. At TP=4, that's 4 heads / 4 ranks = 1 head/rank — valid. At TP=2, that's 4 heads / 2 ranks = 2 heads/rank — the documented invariant is violated, and the recipe author's own fallback (shuffle=0, Triton path) exists precisely to handle that case.
The trigger path: configs/amd-master.yaml (also touched by this PR, just above the diff hunk shown) still defines a live TP2 arm for this exact recipe:
minimaxm3-fp4-mi355x-vllm-agentic-mtp:
...
search-space:
- { tp: 4, kv-offloading: none, conc-list: [...], spec-decoding: mtp }
- { tp: 2, kv-offloading: none, conc-list: [1, 2, 5], spec-decoding: mtp }
- { tp: 4, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.3" }, conc-list: [32, 40], spec-decoding: mtp }Every sweep point in the tp: 2 arm launches minimaxm3_fp4_mi355x_mtp.sh with TP=2, which after this change unconditionally sets VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1. There is nothing else in the script that neutralizes this for TP2 — --attention-backend TRITON_ATTN was already present in the old script alongside the TP-conditional, so it does not make the shuffle var redundant; the shuffle var governs the KV-cache physical layout independently (relevant given the --hf-overrides '{"text_config": {"use_index_cache": true, "index_topk_freq": 4}}' MSA/index-cache path also present in this script).
Why nothing else in this PR prevents it: The PR is otherwise a broad refactor (bash strict-mode fixes, swapping LMCache for vLLM-native KV offload, dropping AIPerf metrics vars, etc.) and the image was bumped to vllm-openai-rocm:v0.27.1. Nothing in the diff or PR description states that the new image relaxes the page-16 fast path's one-head-per-rank requirement — this looks like an accidental drop of a still-necessary guard rather than an intentional simplification, since the guard is a hardware/algorithm invariant tied to MiniMax-M3'''s head count, not an image-version workaround.
Step-by-step proof:
configs/amd-master.yamlschedules a sweep point withtp: 2forminimaxm3-fp4-mi355x-vllm-agentic-mtp.- The launcher sets
TP=2and invokesminimaxm3_fp4_mi355x_mtp.sh. - The script unconditionally executes
export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1(line 129). vllm servestarts with--tensor-parallel-size 2and the AITER page-16 shuffled KV-cache layout enabled.- MiniMax-M3'''s 4 KV heads split across 2 ranks give 2 heads/rank, which the deleted comment states is unsupported by that fast path (only 1 head/rank is supported); the TP4 arm in the same config is the only one satisfying that invariant.
- Depending on how strictly AITER validates this at engine startup, this either fails to launch or produces a silently incorrect KV-cache layout for the TP2 sweep points — either way, those 3 concurrency points (1, 2, 5) in the search space are broken by this change.
The fix: Restore the TP-conditional (if [ "$TP" -eq 4 ]; then ... else ... fi) exactly as it was, or — if the v0.27.1 image genuinely no longer needs the one-head-per-rank restriction — state that explicitly and verify it, since dropping the check silently while keeping the TP2 arm risks a broken or corrupted sweep point either way.
| OFFLOAD_ARGS=( | ||
| --kv-transfer-config | ||
| "{\"kv_connector\":\"LMCacheMPConnector\",\"kv_connector_module_path\":\"lmcache.integration.vllm.lmcache_mp_connector\",\"kv_role\":\"kv_both\",\"kv_connector_extra_config\":{\"lmcache.mp.server_urls\":\"$LMCACHE_SERVER_URLS_CSV\",\"lmcache.mp.mq_timeout\":6000.0}}" | ||
| --kv_offloading_backend native | ||
| --kv_offloading_size "$TOTAL_CPU_DRAM_GB" | ||
| ) | ||
| ;; | ||
| *) | ||
| echo "Unsupported KV_OFFLOAD_BACKEND: $KV_OFFLOAD_BACKEND (expected empty or lmcache)" >&2 | ||
| echo "Unsupported KV_OFFLOAD_BACKEND: ${KV_OFFLOAD_BACKEND:-}" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🔴 The script's KV_OFFLOAD_BACKEND case statement only handles "" and "vllm-native" now (the lmcache branch was removed), but configs/amd-master.yaml's minimaxm3-fp4-mi355x-vllm-agentic-mtp entry still has a third search-space arm using kv-offload-backend: { name: lmcache, version: "0.5.3" } (conc-list [32, 40]). That arm will set KV_OFFLOAD_BACKEND=lmcache, hit the script's *) default branch, and exit 1 with "Unsupported KV_OFFLOAD_BACKEND: lmcache", deterministically failing those two sweep points.
Extended reasoning...
The bug
This PR rewrites benchmarks/single_node/agentic/minimaxm3_fp4_mi355x_mtp.sh to drop LMCache support entirely and replace it with a vllm-native backend (using --kv_offloading_backend native / --kv_offloading_size). The case "${KV_OFFLOAD_BACKEND:-}" statement now only has two real branches: "" (no offload) and vllm-native. Everything else — including the previously-supported lmcache value — falls through to the *) branch, which prints Unsupported KV_OFFLOAD_BACKEND: ... and exit 1.
The triggering config
configs/amd-master.yaml (unchanged by this PR, at line 1701) still has a third search-space arm for minimaxm3-fp4-mi355x-vllm-agentic-mtp:
- { tp: 4, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.3" }, conc-list: [32, 40], spec-decoding: mtp }The code path that triggers it
.github/workflows/run-sweep.ymlsets the workflow inputkv-offload-backend: ${{ matrix.config["kv-offload-backend"].name }}, which for this arm resolves to the literal string"lmcache"..github/workflows/benchmark-tmpl.ymlexports this asKV_OFFLOAD_BACKEND=lmcacheinto the job environment.- The rewritten script's case statement no longer has an
lmcache)branch (it was entirely deleted in this diff, replaced byvllm-native)), soKV_OFFLOAD_BACKEND=lmcachefalls into*). - The script prints
Unsupported KV_OFFLOAD_BACKEND: lmcacheto stderr and callsexit 1.
Why nothing else catches this
There's no validation step between the YAML config and the shell script that cross-checks backend names against what the script actually supports — utils/matrix_logic only validates that kv-offload-backend.name is a non-empty string with an optional version (see test_kv_offload_backend_accepts_optional_version), so lmcache passes that check and reaches the runner, where it fails at execution time instead of at config-validation time.
Step-by-step proof
- Config arm:
tp: 4, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.3" }, conc-list: [32, 40]. - Sweep matrix expansion produces two jobs (conc=32, conc=40) with
KV_OFFLOAD_BACKEND=lmcacheset in the environment. minimaxm3_fp4_mi355x_mtp.shrunscase "${KV_OFFLOAD_BACKEND:-}" in "") ... ;; vllm-native) ... ;; *) echo "Unsupported KV_OFFLOAD_BACKEND: lmcache" >&2; exit 1 ;; esac.- Since
lmcachematches neither""norvllm-native, the*)branch runs, and the script exits 1 before ever starting the vLLM server. - Both sweep points (conc 32 and conc 40) for that arm fail deterministically on every run — this is not a flake.
Before this PR, the script had a working lmcache) branch, so this is a regression introduced specifically by this diff, not a pre-existing issue. The PR description itself only mentions "vllm-simple" DRAM offloading, so the author likely didn't intend to keep the config's lmcache arm active — that arm should either be updated to use vllm-native (matching the new script) or removed if lmcache support is being fully retired for this recipe.
Fix
Either:
- Update the
lmcachearm inconfigs/amd-master.yamlto usekv-offload-backend: { name: vllm-native }(dropping the version field, which is no longer needed), or - Restore an
lmcache)branch in the script if LMCache support should be kept alongside the newvllm-nativepath.
2 participants
Add MiniMax-M3 FP4 MI355X Agentic Support with MTP
Adds an updated single-node agentic-coding benchmark recipe for MiniMax-M3 (FP4) on MI355X using vLLM, with EAGLE3 MTP speculative decoding and DRAM-backed KV cache offloading (vllm-simple).
Changes
minimaxm3-fp4-mi355x-vllm-agentic-mtp(imagevllm-openai-rocm:nightly-cb8104839c141609d99f1254459ef3a4f1bd4263,tp=4, DRAM KV offloading viavllm-simple,spec-decoding: mtp).rejection_sample_method: syntheticwithsynthetic_acceptance_length: 3.35, the committed MiniMax-M3 EAGLE3 golden AL forthinking_onatnum_speculative_tokens=5(golden_al_distribution/minimaxm3_eagle3.yaml), per the AgentX fairness guidelines.EVAL_ONLYaccuracy runs keep real target verification, since synthetic acceptance bypasses verification and corrupts the eval score.Status
Marked [WIP].