[DoNotMerge] Add MiniMax M3 vLLM B300 disagg EAGLE#2182
Conversation
中文:新增 MiniMax M3 NVFP4 在 B300 上的 Dynamo vLLM 分离式 EAGLE Pareto 配置,并使用官方 nightly 镜像与模型名称。
|
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 关于重新运行失败任务的文档 |
1 similar comment
|
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 关于重新运行失败任务的文档 |
中文:移除 MiniMax M3 提交配置中的注释,并更新性能变更日志的 PR 链接。
中文:将 MiniMax M3 Pareto 主配置和全部配方统一改为不带提交哈希的 vLLM nightly 镜像。
| elif [[ $FRAMEWORK == "dynamo-vllm" && $MODEL_PREFIX == "minimaxm3" && $PRECISION == "fp4" ]]; then | ||
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
| mkdir -p recipes/vllm/minimax-m3 | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3" recipes/vllm/minimax-m3 | ||
| elif [[ $FRAMEWORK == "dynamo-vllm" && $MODEL_PREFIX == "minimaxm3" && $PRECISION == "fp8" ]]; then | ||
| git clone https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
| git checkout sa-submission-q2-2026 | ||
| mkdir -p recipes/vllm/minimax-m3 | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3" recipes/vllm/minimax-m3 | ||
| if [[ $PRECISION == "fp8" ]]; then | ||
| SRTCTL_SETUP_SCRIPT="minimax-m3-vllm-fixes.sh" | ||
| fi | ||
| SRTCTL_SETUP_SCRIPT="minimax-m3-vllm-fixes.sh" | ||
| # NVIDIA/srt-slurm#38 | ||
| git show 22d46ba9971615016d2339c9ffbc7b4597accfad --format= -- src/srtctl/core/ip_utils/get_node_ip.sh | git apply - || exit 1 |
There was a problem hiding this comment.
🔴 The new fp4-only branch in runners/launch_b300-nv.sh (lines 79-92) is keyed only on MODEL_PREFIX==minimaxm3 && PRECISION==fp4 && FRAMEWORK==dynamo-vllm, which also matches the pre-existing, unrelated minimaxm3-fp4-b300-dynamo-vllm sweep in configs/nvidia-master.yaml (different image, router 1.3.0.dev20260614, recipes under b300-fp4/1k1k and 8k1k). That existing sweep is silently rerouted from the pinned sa-submission-q2-2026 branch (with the NVIDIA/srt-slurm#38 get_node_ip.sh patch) to unpinned srt-slurm main with no patch, and the same unscoped condition at line 187 also strips --no-preflight validation from it. Please scope both changes to the new MTP config (e.g. by CONFIG_FILE, router version, or image) so the pre-existing sweep is unaffected.
Extended reasoning...
What the bug is: The PR splits the previous combined elif [[ $FRAMEWORK == "dynamo-vllm" && $MODEL_PREFIX == "minimaxm3" && ( $PRECISION == "fp4" || $PRECISION == "fp8" ) ]] branch into two separate branches — one for fp4 and one for fp8. The new fp4-only branch was rewritten to support the new MTP/EAGLE3 recipe set this PR introduces, but the condition that selects it (FRAMEWORK==dynamo-vllm && MODEL_PREFIX==minimaxm3 && PRECISION==fp4) is not specific enough to distinguish "the new MTP sweep" from "any minimaxm3 fp4 dynamo-vllm sweep" — including the pre-existing, unrelated minimaxm3-fp4-b300-dynamo-vllm config already defined in configs/nvidia-master.yaml:12951.
The code path that triggers it: configs/nvidia-master.yaml:12951 defines minimaxm3-fp4-b300-dynamo-vllm with model-prefix: minimaxm3, precision: fp4, framework: dynamo-vllm — exactly the three keys the launcher branches on. It uses its own image (vllm-minimax-m3-perf-x86_64-13.0.1-8b00f41), its own router version (1.3.0.dev20260614), and recipes under b300-fp4/1k1k and b300-fp4/8k1k — none of which were touched by this PR. Before this PR, when that config ran, the launcher's combined fp4||fp8 branch cloned NVIDIA/srt-slurm, ran git checkout sa-submission-q2-2026, and unconditionally applied the NVIDIA/srt-slurm#38 get_node_ip.sh patch (only SRTCTL_SETUP_SCRIPT was fp8-gated). After this PR, that same config now falls into the new fp4-only branch (lines 79-84), which clones --branch main --single-branch with no checkout and no patch applied. Separately, the unscoped guard added at lines 187-189 (if [[ MODEL_PREFIX == minimaxm3 && PRECISION == fp4 ]]; then SRTCTL_APPLY_ARGS+=(--no-preflight); fi) also fires for this same pre-existing config, silently disabling srtctl's preflight validation for a sweep that previously ran with preflight enabled.
Why existing code doesn't prevent it: The launcher routes purely on MODEL_PREFIX/PRECISION/FRAMEWORK, which is shared, coarse-grained infrastructure — it has no way to distinguish "old fp4 sweep" from "new MTP fp4 sweep" using those three keys alone, since both configs share all three values. Nothing in the diff adds a discriminator (e.g. CONFIG_FILE path, router version, or image) to scope the new git-clone/patch logic or the --no-preflight flag to just the new MTP recipes.
Impact: If the pre-existing minimaxm3-fp4-b300-dynamo-vllm sweep is triggered after this PR merges, it silently builds against an unpinned, unvalidated srt-slurm main ref instead of the previously-tested sa-submission-q2-2026 branch, and loses the get_node_ip.sh IP-resolution patch it depended on. It also now skips srtctl's preflight node/health validation. Depending on whether main has diverged from sa-submission-q2-2026 in ways incompatible with the old recipe/setup flow, or lacks the #38 fix, this can cause node-IP resolution failures or let a bad node config slip past preflight and fail mid-run, wasting cluster time on the shared B300 runner.
How to fix: Scope the new branch condition (lines 79-84) and the --no-preflight guard (lines 187-189) more narrowly than MODEL_PREFIX/PRECISION/FRAMEWORK alone — e.g. gate on the CONFIG_FILE path (*-mtp recipes), the router version, or a dedicated env var passed from the new minimaxm3-fp4-b300-dynamo-vllm-mtp config key, so the old sweep keeps its original clone/patch/preflight behavior untouched.
Step-by-step proof:
configs/nvidia-master.yaml:12951definesminimaxm3-fp4-b300-dynamo-vllmwithmodel-prefix: minimaxm3,precision: fp4,framework: dynamo-vllm.- When this config's sweep runs, the launcher (
runners/launch_b300-nv.sh) evaluatesMODEL_PREFIX=minimaxm3,PRECISION=fp4,FRAMEWORK=dynamo-vllmagainst the elif chain. - The new condition at line 79,
[[ $FRAMEWORK == "dynamo-vllm" && $MODEL_PREFIX == "minimaxm3" && $PRECISION == "fp4" ]], evaluates true for this pre-existing config exactly as it does for the new MTP config — there is no key that differs between them in this condition. - Execution enters lines 80-83:
git clone --branch main --single-branch ..., nogit checkout sa-submission-q2-2026, nogit show ... | git applyfor [NVIDIA] Add SGL B200 FP4 docker script #38. - Separately, at line 187,
[[ "$MODEL_PREFIX" == "minimaxm3" && "$PRECISION" == "fp4" ]]is also true, so--no-preflightis appended toSRTCTL_APPLY_ARGSfor this same run. - Net effect for a sweep the PR never intended to touch: it now builds against
srt-slurm main(unpinned, no [NVIDIA] Add SGL B200 FP4 docker script #38 patch) and applies without srtctl preflight checks — a behavior change with no test coverage for the old recipe set againstmain.
| resources: | ||
| gpu_type: "b300" | ||
| gpus_per_node: 8 | ||
| prefill_nodes: 1 | ||
| decode_nodes: 1 | ||
| prefill_workers: 2 | ||
| decode_workers: 1 | ||
| gpus_per_prefill: 2 | ||
| gpus_per_decode: 4 |
There was a problem hiding this comment.
🟡 In this new 2p1d-dep2-dep4-eagle3-8k1k.yaml recipe, decode_nodes: 1 reserves a second b300 node even though the topology (2×DEP2 prefill + 1×DEP4 decode = 8 GPUs) fits exactly within one 8-GPU node, and colocation is enabled. Every other sibling recipe added in this same PR (e.g. 1p1d-dep2-dep4-eagle3-8k1k.yaml and 1p3d-tp2-tp2-eagle3-8k1k.yaml) sets decode_nodes: 0 for an equal-or-larger single-node-fitting topology, so this looks like a copy-paste leftover from the DEP8-decode variant it was likely derived from.
Extended reasoning...
The bug. benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3/b300-fp4/8k1k/mtp/2p1d-dep2-dep4-eagle3-8k1k.yaml sets prefill_nodes: 1, decode_nodes: 1, prefill_workers: 2 (2 GPUs each → 4 GPUs) and decode_workers: 1 (4 GPUs) — total 8 GPUs. gpus_per_node: 8, so the whole topology fits in exactly one b300 node, and both allow_prefill_decode_colocation and allow_prefill_decode_colocation_across_nodes are true, meaning decode is explicitly permitted to share the prefill node. prefill_nodes + decode_nodes = 2 therefore reserves a second, entirely unused b300 node (16 GPUs reserved for an 8-GPU job).\n\nEvidence this is a real inconsistency and not just a stylistic quirk. Within this exact PR, 8 sibling recipes are added in the same mtp/ directory, and every one of them follows a strict rule: prefill_nodes + decode_nodes == ceil(total_gpus / gpus_per_node) given the packing enabled by the colocation flags. Concretely: 1p1d-dep2-dep4 (2+4=6 GPUs) → decode_nodes=0; 1p3d-tp2-tp2 (2+6=8 GPUs) → decode_nodes=0; 1p2d-tp4-tp4 (4+8=12) → decode_nodes=1 (1+1=2 nodes, correct since 12>8); 1p7d-tp4-tp4 (4+28=32) → decode_nodes=3 (4 nodes total, correct). The 2p1d recipe is the only one of the nine new files that breaks this pattern — its 8-GPU total should map to 1 total node (decode_nodes=0), matching its direct sibling 1p1d-dep2-dep4 and 1p3d-tp2-tp2, both of which also land exactly at a node boundary.\n\nLikely root cause. The existing (pre-PR) 1k1k recipe 2p1d-dep2-dep8-1k1k.yaml has the same worker shape (2×DEP2 prefill + 1 decode worker) but gpus_per_decode: 8, so its decode worker legitimately cannot fit in the prefill node's remaining 4 GPUs and correctly needs decode_nodes: 1. This new recipe appears to have been derived from that file by shrinking the decode worker from DEP8 to DEP4, but decode_nodes was left at 1 instead of being dropped to 0 — a copy-paste-style leftover.\n\nAddressing the counterargument. One reviewer pointed out that an older, unrelated recipe in this repo — b300-fp4/8k1k/1p1d-dep2-tp4-8k1k.yaml (not touched by this PR, added earlier) — has an identical GPU layout (2+4=6 GPUs, one-node-fittable, allow_prefill_decode_colocation: true) yet also sets decode_nodes: 1, arguing this is 'the norm' rather than a bug. That file is real and does contradict the ceil-based rule too — but it's evidence of a pre-existing, unrelated inconsistency elsewhere in the repo, not evidence that this PR's own internal convention (which every other new file in this PR follows exactly) was intentional here. The stronger, more local signal is that this PR's own freshly-authored, directly-adjacent sibling (1p1d-dep2-dep4-eagle3-8k1k.yaml) uses decode_nodes=0 for a smaller topology, making the 2p1d file the clear outlier among files introduced in this very diff.\n\nImpact and fix. This does not crash the job or corrupt the benchmark result — the reported GPU/worker counts used for the sa-bench metrics come from prefill_workers/decode_workers × gpus_per_* in the master config, not from the node reservation. The concrete cost is wasted cluster allocation (an extra b300 node sits idle for the duration of the sweep) and, if allow_prefill_decode_colocation_across_nodes causes the scheduler to actually spread prefill and decode across the two reserved nodes instead of packing them, it could shift KV transfer from intra-node NVLink to inter-node UCX/NIXL, changing the measured latency profile relative to the intended single-node topology. The fix is simply to set decode_nodes: 0 in this file, consistent with its sibling 1p1d-dep2-dep4-eagle3-8k1k.yaml and 1p3d-tp2-tp2-eagle3-8k1k.yaml.
No description provided.