[NV] minimaxm3-fp8-gb200-dynamo-vllm-mtp: - #2432
Conversation
…ix non-MTP comments - Add minimaxm3-fp8-gb200-dynamo-vllm-mtp: two EAGLE3 SLURM recipes (1p2d-dep4-tep8 conc 16/64/128, 2p1d-dep4-dep16 conc 512/1024) under minimax-m3-gb200-fp8/8k1k/mtp/; launcher already cp-rT's the whole minimax-m3-gb200-fp8 tree so no runner changes needed - Fix minimaxm3-fp8-gb200-dynamo-vllm: correct 4 wrong comments (Xp2d DEP8 -> Xp1d DEP16), remove phantom Marlin entry from header, bump router version 1.3.0.dev20260614 -> 1.3.0.dev20260710 中文：新增 MiniMax M3 MXFP8 GB200 Dynamo-vLLM EAGLE3 投机解码配方并修正 非 MTP 配置注释 - 新增 minimaxm3-fp8-gb200-dynamo-vllm-mtp：两个 EAGLE3 SLURM 配方 （1p2d-dep4-tep8 并发 16/64/128，2p1d-dep4-dep16 并发 512/1024）， 位于 minimax-m3-gb200-fp8/8k1k/mtp/；启动器已通过 cp-rT 复制整个 minimax-m3-gb200-fp8 目录，无需修改 runner 脚本 - 修正 minimaxm3-fp8-gb200-dynamo-vllm：修正 4 处错误注释 （Xp2d DEP8 -> Xp1d DEP16），移除头注释中不存在的 Marlin 条目， 将 router 版本从 1.3.0.dev20260614 升级至 1.3.0.dev20260710 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.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 关于重新运行失败任务的文档 |
…m-mtp 中文：补全 minimaxm3-fp8-gb200-dynamo-vllm-mtp 的 perf-changelog pr-link Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
xinli-sw
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - "Add MiniMax M3 MXFP8 GB200 Dynamo-vLLM disaggregated EAGLE3 MTP recipes" | ||
| - "Fix comment errors in minimaxm3-fp8-gb200-dynamo-vllm (Xp2d DEP8 -> Xp1d DEP16, remove phantom Marlin entry, bump router to 1.3.0.dev20260710)" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2432 |
There was a problem hiding this comment.
🔴 The appended changelog entry uses pr-link: TBD, but the changelog tooling only recognizes XXX or the canonical .../pull/<n> URL as valid pr-link values (see utils/validate_perf_changelog.py and utils/prepare_perf_changelog_merge.py). This will raise ChangelogValidationError when merge_with_reuse.sh runs prepare_perf_changelog_merge.py canonicalize at merge time, blocking the standard merge path until manually fixed. Please change TBD to XXX (or the real PR URL).
Extended reasoning...
The newly appended entry at the end of perf-changelog.yaml (for minimaxm3-fp8-gb200-dynamo-vllm-mtp) sets:
pr-link: TBDThe changelog tooling only accepts two forms for an unmerged/appended entry's pr-link:
- A placeholder from
PR_LINK_PLACEHOLDERSinutils/validate_perf_changelog.py(lines ~24-27), which is{"XXX", "https://github.com/SemiAnalysisAI/InferenceX/pull/XXX"}. - The canonical
https://github.com/SemiAnalysisAI/InferenceX/pull/<pr_number>URL.
"TBD" matches neither. Note that the PR diff as displayed by GitHub/CI tooling shows the link already canonicalized to .../pull/2432, but that's an artifact of display-time canonicalization in this review pipeline — the actual committed content on this branch (confirmed via git show HEAD:perf-changelog.yaml) is pr-link: TBD.
Why this isn't caught before merge: The PR-time CI gate (validate_perf_changelog.py invoked from the changelog-check workflow) only calls validate_matrix_compatible_change, which drives matrix generation via process_changelog.py. It never calls validate_added_pr_link/compare_entries, so an invalid placeholder like TBD sails through CI silently.
Where it actually breaks: The standard merge path, merge_with_reuse.sh <pr>, unconditionally runs prepare_perf_changelog_merge.py canonicalize. That script's canonicalize_appended_links function (lines ~109-114) checks each appended entry's pr-link against PR_LINK_PLACEHOLDERS and the expected canonical URL; since TBD matches neither, it raises:
ChangelogValidationError: appended entry N has unexpected pr-link 'TBD'
Step-by-step proof:
- Author appends the changelog entry with
pr-link: TBD(perf-changelog.yaml:5356). - PR CI's changelog check only validates matrix compatibility, not the pr-link format — the PR shows green.
- Someone runs
utils/merge_with_reuse.sh 2432to merge, which internally invokespython utils/prepare_perf_changelog_merge.py canonicalize .... canonicalize_appended_linksiterates appended entries, reaches this one, seespr-link == "TBD", which is not inPR_LINK_PLACEHOLDERS = {"XXX", ".../pull/XXX"}and does not equal the expectedhttps://github.com/SemiAnalysisAI/InferenceX/pull/2432.- The function raises
ChangelogValidationError, aborting the merge script before it can canonicalize the link and complete the merge. - A maintainer has to manually edit
perf-changelog.yamlto replaceTBDwithXXX(or the real URL) before retrying the merge.
This is also confirmed as an outlier: a grep of the entire 5356-line file shows only one non-empty, non-URL pr-link value (TBD, this entry) — every other unmerged entry in the file consistently uses XXX.
Fix: change pr-link: TBD to pr-link: XXX (the standard placeholder used everywhere else in the file), which merge_with_reuse.sh will then correctly canonicalize to the real PR URL at merge time.
| osl: 1024 | ||
| search-space: | ||
| # 1p2d DEP4+DEP8, 5n: conc 512 | ||
| # 1p1d DEP4+DEP16, 5n: conc 512 |
There was a problem hiding this comment.
🟡 This PR corrects the stale "Xp2d DEP8" comment wording to "Xp1d DEP16" in configs/nvidia-master.yaml, but the sibling SLURM recipe files it references (1p1d, 2p1d, 3p1d, 5p1d-dep4-dep16-8k1k.yaml under benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3-gb200-fp8/8k1k/) still carry the identical stale header comment on line 3. It's comment-only and doesn't affect behavior, but since these files are what CONFIG_FILE points at, fixing them alongside the master-config comments would keep the same bug class from lingering.
Extended reasoning...
This PR explicitly sets out to fix a class of stale comments in configs/nvidia-master.yaml where search-space entries were mislabeled Xp2d DEP8 (implying 2 decode workers each running DEP8) when the actual resources block shows a single DEP16 decode worker (decode_workers: 1, gpus_per_decode: 16). The PR correctly rewrites these comments to Xp1d DEP16 at lines 8081, 8126, 8141, and 8156 of configs/nvidia-master.yaml.
However, the four sibling SLURM recipe YAML files that these minimaxm3-fp8-gb200-dynamo-vllm search-space entries reference via CONFIG_FILE=recipes/vllm/minimax-m3-gb200-fp8/8k1k/{1p1d,2p1d,3p1d,5p1d}-dep4-dep16-8k1k.yaml still contain the exact same stale wording in their own header comments. For example, 1p1d-dep4-dep16-8k1k.yaml line 3 reads:
# 1P DEP4 prefill (TP1 DP4 EP, 4 GPU/worker) + 2D DEP8 decode (TP1 DP8 EP, 8 GPU/worker = 2 nodes each).
yet the file's own resources block a few lines down shows decode_workers: 1 and gpus_per_decode: 16, and the vllm_config.decode section shows tensor-parallel-size: 1, data-parallel-size: 16 — i.e. one worker running DEP16, not two workers each running DEP8 across 2 nodes. The same mismatch (stale "2D DEP8 / 2 nodes each" header vs. the real single DEP16 worker config) appears identically in 2p1d-dep4-dep16-8k1k.yaml, 3p1d-dep4-dep16-8k1k.yaml, and 5p1d-dep4-dep16-8k1k.yaml.
Why this wasn't caught by existing validation: nothing in the config loader or CI cross-checks free-text YAML comments against the structured resources/vllm_config fields they describe, so a comment can drift arbitrarily from the config it documents without any test failing — which is exactly how the original bug in nvidia-master.yaml was introduced and now persists here in the recipe files.
Proof/walkthrough: Open benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3-gb200-fp8/8k1k/1p1d-dep4-dep16-8k1k.yaml. Line 3 says "2D DEP8 decode ... 8 GPU/worker = 2 nodes each" — describing 2 decode workers, 8 GPUs each. But scroll to the resources: block: decode_workers: 1, gpus_per_decode: 16 — 1 worker, 16 GPUs. Scroll further to backend.vllm_config.decode: tensor-parallel-size: 1, data-parallel-size: 16 — confirms DEP16 with a single worker, matching the corrected # 1p1d DEP4+DEP16 comment this PR now writes into nvidia-master.yaml for the referencing search-space entry. The same check applies to the 2p1d/3p1d/5p1d variants.
Impact: Purely cosmetic/documentation — none of these fields feed into runtime behavior, so nothing breaks. But it undermines the very consistency this PR is trying to restore, since a reader following the CONFIG_FILE reference from the now-correct nvidia-master.yaml comment will land on a recipe file whose own header still describes the old, wrong topology.
Fix: Update line 3 of each of the four sibling files to match the corrected wording, e.g. # 1P DEP4 prefill (TP1 DP4 EP, 4 GPU/worker) + 1D DEP16 decode (TP1 DP16 EP, 16 GPU/worker).
| max-model-len: 9472 | ||
| language-model-only: true | ||
| kv-cache-dtype: fp8 | ||
| speculative-config: '{"method":"eagle3","model":"Inferact/MiniMax-M3-EAGLE3-GQA","num_speculative_tokens":3,"attention_backend":"FLASH_ATTN"}' |
There was a problem hiding this comment.
🟡 In the two new EAGLE3 MTP recipes, the prefill speculative-config sets num_speculative_tokens: 3 (same as decode), whereas all 6 sibling MiniMax-M3 EAGLE3 MTP recipes on B300 — which this PR explicitly adapts from (PR #1863) — use prefill=1/decode=3. Both new GB200 files (1p2d-dep4-tep8-eagle3-8k1k.yaml:74 and 2p1d-dep4-dep16-eagle3-8k1k.yaml:74) are affected; worth a quick confirmation this was intentional rather than a copy-paste of the decode block.
Extended reasoning...
Both new GB200 MTP recipes set num_speculative_tokens: 3 in the prefill speculative-config block, exactly matching the decode block's value. Checking every existing MiniMax-M3 EAGLE3 MTP recipe on B300 (benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3/b300-fp4/8k1k/mtp/*.yaml, 6 files: 1p1d, 1p2d, 1p4d, 1p6d, 2p1d, 2p3d) shows a uniform pattern of prefill=1, decode=3 — no exception. The configs/nvidia-master.yaml header comment for this exact sweep says it was "adapted from NV B300 PR #1863," making the B300 recipes the direct template these new GB200 files are supposed to mirror.
The affected locations are 1p2d-dep4-tep8-eagle3-8k1k.yaml:74 and 2p1d-dep4-dep16-eagle3-8k1k.yaml:74 — both set prefill num_speculative_tokens to 3 instead of the established 1.
Step-by-step check:
grep -n num_speculative_tokensacross the 6 B300 sibling files shows every prefill line reads"num_speculative_tokens":1and every decode line reads"num_speculative_tokens":3— 6/6 consistent.- The new PR's
1p2d-dep4-tep8-eagle3-8k1k.yamlline 74 (prefill block) reads"num_speculative_tokens":3— identical to its own decode block's value at the corresponding line further down. - Same pattern repeats in
2p1d-dep4-dep16-eagle3-8k1k.yamlline 74. - This is the exact shape you'd expect from copy-pasting the decode
speculative-configstring into the prefill block during recipe authoring.
That said, this is not a clear-cut bug and shouldn't block merge. A counter-precedent exists elsewhere in the repo: DeepSeek-V4 disaggregated MTP recipes (e.g. deepseek-v4/8k1k/disagg-gb200-low-latency-mtp2.yaml) set the same num_speculative_tokens on both prefill and decode (both 2), so "prefill == decode" is itself a legitimate, precedented convention for spec-decoding recipes in this repo — just not the one MiniMax-M3 has followed so far. Functionally, in disaggregated P/D serving the prefill worker performs a single prefill pass and hands off KV; it does not run the iterative multi-token speculative-decode loop the way the decode worker does, so this prefill setting mostly governs draft-model resource/cudagraph allocation rather than output correctness. There's no indication merging as-is would crash or corrupt results — worst case is a benign resource-allocation difference that the sweep (labeled full-sweep-fail-fast) will exercise empirically.
Suggested fix: if the intent was to mirror the B300 template, change prefill num_speculative_tokens to 1 in both new files at line 74. If prefill==decode was a deliberate choice (following the DeepSeek-V4 pattern instead), no change is needed — just worth a one-line confirmation from the author.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30579166219 |
Both EAGLE3 MTP recipe YAMLs and master-config tp/ep updated. 中文：将 GB200 fp8 MTP 预填充并行从 TP1 DP4 改为 TP2 DP2， 同步更新两个 EAGLE3 MTP 配方 YAML 及主配置 tp/ep 字段 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
中文：合并 origin/main，重新追加 perf-changelog 条目 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
中文：将 GB200 fp8 MTP 镜像更新为 vllm/vllm-openai:v0.26.0 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30588207308 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30590493633 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30590493633 |
1 participant
No description provided.