Update vLLM MiniMax-M3 image tags / 更新 vLLM MiniMax-M3 镜像标签 - #2120
Update vLLM MiniMax-M3 image tags / 更新 vLLM MiniMax-M3 镜像标签#2120wzhao18 wants to merge 14 commits into
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 关于重新运行失败任务的文档 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| - config-keys: | ||
| - minimaxm3-fp4-b300-vllm | ||
| description: | ||
| - "update vllm image" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2120 |
There was a problem hiding this comment.
🟡 This PR is materially more than a image bump, but the new perf-changelog entry (perf-changelog.yaml:4623-4628) only says "update vllm image". Per AGENTS.md §"Updating Docker images", each material change should be its own bullet — please also list the new VLLM_MINIMAX_M3_BF16_ROUTER_GEMM=1 env var (with the bf16-router-gate weight patch it enables), the pinned FlashInfer nightly-v0.6.14-20260702 + CuTeDSL split-K gemm patch, and the new --attention_config.indexer_kv_dtype fp8 CLI arg. Also, before dropping the [WIP] marker, please make the PR title bilingual per AGENTS.md (<English title> / <中文标题>).
Extended reasoning...
What the finding is. The perf-changelog entry appended in this PR (perf-changelog.yaml:4623-4628) has a single-bullet description update vllm image, but the PR is doing five distinct things beyond an image bump:
- Sets
VLLM_MINIMAX_M3_BF16_ROUTER_GEMM=1in the recipe (benchmarks/single_node/fixed_seq_len/minimaxm3_fp4_b300.sh:92). - Applies
patches/minimax-m3-bf16-router-gemm.patchto the installed vLLM, which adds that env var tovllm/envs.pyand, when set, switchesMiniMaxM3MoE.gate'sGateLinearparams_dtypefromtorch.float32totorch.bfloat16. That's a real numerical change: instead of upcasting bf16 activations to fp32 and running an fp32 gate, the weights are now rounded fp32→bf16 and the gate runs cuBLAS bf16 × bf16 → fp32. - Installs FlashInfer
0.6.14.dev20260702(nightly-v0.6.14-20260702) explicitly on top of the image, replacing the version baked into the container. - Applies a ~3100-line
flashinfer-cutedsl-splitk-gemm.patchto that installed FlashInfer (new CuTeDSL split-K kernel for low-M MXFP8 gemms). - Adds
--attention_config.indexer_kv_dtype fp8to thevllm serveinvocation (recipe line 112).
Why the convention matters here. AGENTS.md §"Updating Docker images" gives an explicit example showing that an image bump paired with a new env var should be TWO bullets, not one:
description:
- 'Update vLLM image from v0.11.2 to v0.13.0'
- 'Add VLLM_MXFP4_USE_MARLIN=1 environment variable'
The changelog description is what surfaces on inferencex.com next to the perf delta this PR produces. If any of the five items above moves the numbers — and the bf16-router weight rounding in particular is a change to numerical semantics of the routing GEMM, not a no-op — a reader seeing only "update vllm image" will attribute the delta entirely to the image swap. Sibling entries in the same file (e.g. the glm5 configs above at ~4600-4602 and dsr1 at ~4619-4621) all follow the multi-bullet convention.
Step-by-step proof for the bf16 router GEMM entry alone. In patches/minimax-m3-bf16-router-gemm.patch, the GateLinear construction in vllm/models/minimax_m3/nvidia/model.py changes from params_dtype=torch.float32 to params_dtype=torch.bfloat16 if envs.VLLM_MINIMAX_M3_BF16_ROUTER_GEMM else torch.float32. The recipe sets that env var to 1, so on every run the router gate weights are the bf16-rounded version of the checkpoint's fp32 weights. That's a behavior change independent of the image bump, and it's the kind of item AGENTS.md's example says to list as its own bullet.
Bilingual PR title. Separately, AGENTS.md line 7 states "PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format: <English title> / <中文标题>. … This applies to every PR and every issue." The current title [WIP] Update vllm B300 Minimax M3 is English-only. One verifier reasonably observed that this is PR metadata on a WIP PR and would normally be finalized when the WIP marker is dropped — that's fine; the note is just a reminder to update the title before requesting review, consistent with how recent MiniMax-M3 image bumps (#2107, #2003) landed.
How to fix. Expand the changelog description to something like:
- 'Update MiniMax-M3 FP4 B300 vLLM image (…→ nightly-2afa3f7e9502…)'
- 'Install pinned FlashInfer nightly-v0.6.14-20260702 and apply the CuTeDSL split-K gemm patch'
- 'Apply MiniMax-M3 bf16-router GEMM patch and enable it via VLLM_MINIMAX_M3_BF16_ROUTER_GEMM=1'
- 'Add --attention_config.indexer_kv_dtype fp8 to vllm serve'
and update the PR title to Update vllm B300 Minimax M3 / <中文标题> before dropping [WIP].
Severity. Nit — this is a documentation completeness issue on a WIP PR, not a runtime defect. Flagging it now so the changelog can be expanded before the [WIP] marker is dropped.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28972702517 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28973507570 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28976585567 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28978689802 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28978821621 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28978901633 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29624967726 |
| SITE_PACKAGES=$(dirname "$(python3 -c "import importlib.util; print(importlib.util.find_spec('flashinfer').submodule_search_locations[0])")") \ | ||
| || { echo "Could not locate the installed flashinfer package" >&2; exit 1; } |
There was a problem hiding this comment.
🟡 In minimaxm3_fp4_b300.sh:56-57, SITE_PACKAGES=$(dirname "$(python3 -c ...)") || { echo ...; exit 1; } only checks the exit status of the outer dirname, not the inner python3 command substitution. If python3 fails to locate flashinfer, dirname "" still succeeds and prints ".", so the error guard never fires and SITE_PACKAGES silently becomes ".". Fix by capturing the python3 output into its own variable (checked with ||) before passing it to dirname.
Extended reasoning...
What the bug is. On line 56, SITE_PACKAGES$(dirname "$(python3 -c ...)") nests a python3 command substitution inside a dirname command substitution, then guards the whole assignment with || { echo "Could not locate the installed flashinfer package" >&2; exit 1; }. In bash, the exit status of VAR=$(cmd) is the exit status of cmd — here, dirname, not the nested python3 invocation. If python3 -c "...find_spec('flashinfer').submodule_search_locations[0]..." fails (flashinfer not importable, or find_spec returns None causing an AttributeError on .submodule_search_locations), it exits non-zero and prints nothing to stdout. But dirname "" (dirname of an empty string) itself prints "." and exits 0. The outer || therefore never fires, and SITE_PACKAGES is silently set to "." instead of triggering the intended fail-fast error message.\n\nStep-by-step proof. This can be reproduced directly:\n\nSITE_PACKAGES=$(dirname "$(python3 -c 'import sys; sys.exit(1)')") || echo caught\necho "SITE_PACKAGES=[$SITE_PACKAGES]"\n\nThis does NOT print "caught", and prints SITE_PACKAGES=[.] — confirming the inner python3 failure is fully swallowed by the outer dirname call's own success.\n\nWhy existing guards don't fully prevent it. The immediately preceding lines do install flashinfer with their own || exit 1 guard (the pip install a few lines above), which makes reaching this specific find_spec failure unlikely in the common case — flashinfer should be importable by the time this line runs. And even if SITE_PACKAGES did end up ".", the following line, patch -p1 -d "${SITE_PACKAGES}" < "$FLASHINFER_PATCH", would try to patch flashinfer/gemm/gemm_base.py relative to the script's cwd, which doesn't exist there, so patch itself fails and its own || { echo ...; exit 1; } guard aborts the script anyway. So the net runtime behavior is still a fail-fast abort — just with a less accurate/misleading diagnostic message ("FlashInfer CuTeDSL split-K gemm patch failed" instead of "Could not locate the installed flashinfer package").\n\nContrast with the codebase's own convention. The sibling script minimaxm3_fp8_b300.sh resolves the same kind of path using a single-level $(...) capture around a heredoc that explicitly does if spec is None or not spec.submodule_search_locations: sys.exit(1), then applies its own || exit 1 directly to that capture — no nested command substitution masking the exit status. This PR's script diverges from that established, correctly fail-safe pattern.\n\nFix. Split the resolution into two steps so the || guard actually observes python3's exit status:\n\nFLASHINFER_PKG_DIR=$(python3 -c "import importlib.util; print(importlib.util.find_spec('flashinfer').submodule_search_locations[0])") \\n || { echo "Could not locate the installed flashinfer package" >&2; exit 1; }\nSITE_PACKAGES=$(dirname "$FLASHINFER_PKG_DIR")\n\n\nSeverity rationale. This is a genuine error-handling defect, but not blocking: the trigger condition is nearly unreachable given the preceding guarded pip install, and even in the masked-failure case the script still fails fast via the next patch command's own guard rather than silently continuing to a wrong/successful outcome. It's a robustness/diagnostics-quality issue rather than a functional regression, so it should be fixed but doesn't need to block merge.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29625059994 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29932104401 |
2 similar comments
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29932104401 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29932104401 |
- Bump all four fp4 vLLM images to nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9 - Remove VLLM_FLASHINFER_ALLREDUCE_BACKEND=trtllm from B200/B200-mtp/B300-mtp scripts - Add --attention_config.indexer_kv_dtype fp8 to B200 non-eagle config Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # perf-changelog.yaml
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29984401390 |
wzhao18
changed the title
|
/reuse-sweep-run 29984401390 |
|
As a PR reviewer and CODEOWNER, I have reviewed this and have:
Additional detail section:
Signed: |
❌❌❌ REJECTED ❌❌❌@Ankur-singh — blocking: ✅ Check 0 (CODEOWNER): PASS — |
|
recipe. vllm-project/recipes#682 |
Ankur-singh
changed the title
|
As a PR reviewer and CODEOWNER, I have reviewed this and have:
Additional detail section:
Signed: 中文：
|
✅✅✅ Verdict: PASS ✅✅✅✅ Check 0 (CODEOWNER): PASS — |
|
@Ankur-singh What is the plan for merging this one? |
6 participants
Summary
vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9.VLLM_FLASHINFER_ALLREDUCE_BACKEND=trtllmfrom all four launchers.--attention_config.indexer_kv_dtype fp8.perf-changelog.yaml.Validation
中文说明
vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9。VLLM_FLASHINFER_ALLREDUCE_BACKEND=trtllm。--attention_config.indexer_kv_dtype fp8将独立的 MSA indexer KV cache 设置为 FP8。perf-changelog.yaml。验证