[WIP] Update vllm B300 Minimax M3#2124
Conversation
jiahanc
requested review from
Ankur-singh,
jgangani and
kedarpotdar-nv
as code owners
|
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 关于重新运行失败任务的文档 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # baked into the perf container image. | ||
| # | ||
| # At runtime the recipe swaps the image's FlashInfer for the pinned nightly | ||
| # release and applies the CuTeDSL split-K gemm patch on top, mirroring vLLM | ||
| # commits 82a00090a (nightly install) and 0a16bea6b (patch). |
There was a problem hiding this comment.
🟡 The header comment (lines 6-10) says the recipe swaps in the pinned FlashInfer nightly and applies the CuTeDSL split-K gemm patch at runtime, but the entire block that would do this (lines 26-60) is prefixed with # # — nothing runs at benchmark time. The new 3115-line patches/flashinfer-cutedsl-splitk-gemm.patch is only referenced from inside that commented-out block, so it's currently dead weight in the tree. Before flipping this out of [WIP], either uncomment the install/patch block so the header matches actual behavior, or drop the misleading header lines and the patch file.
Extended reasoning...
What the mismatch is
The updated header comment at lines 7-10 explicitly asserts: "At runtime the recipe swaps the image's FlashInfer for the pinned nightly release and applies the CuTeDSL split-K gemm patch on top, mirroring vLLM commits 82a00090a (nightly install) and 0a16bea6b (patch)." That is the sole documentation a future reader has for what this recipe actually runs.
The block that would implement that behavior spans lines 26-60. Every non-blank line in it is prefixed with # (the outer # # visible in the diff is the two-level commenting — a block comment plus the original inline # shell comments). Because the shell simply sees #… on every line, none of these commands execute: no pip uninstall, no pip install of the nightly wheels, no patch -p1. The recipe runs against whatever FlashInfer the container image happens to ship.
Concrete proof, step-by-step
source .../benchmark_lib.shruns (line 12).check_env_vars ...runs (lines 14-23).- Line 26 is
# # --- FlashInfer nightly + CuTeDSL split-K gemm patch .... Bash treats# ...as a comment — the# #prefix is just two hash characters at column 0. - Lines 27-60 all start with
#(verified in the diff and in the modified-files view). Bash silently skips each one. - Execution resumes at line 62 (
if [[ -n "${MODEL_PATH:-}" ]]; then).FLASHINFER_VERSION,FLASHINFER_PATCH,SITE_PACKAGES, etc. are never set;pip uninstall/pip install/patchare never invoked. vllm serveat line 96 launches with the image's pre-baked FlashInfer — no nightly, no split-K patch.
Dead-file consequence
The only two references to patches/flashinfer-cutedsl-splitk-gemm.patch in the repo are inside that commented-out block (line 49: # FLASHINFER_PATCH="…/patches/flashinfer-cutedsl-splitk-gemm.patch", line 55: # patch -p1 -d "${SITE_PACKAGES}" < "$FLASHINFER_PATCH"). Since neither line executes, nothing in the codebase reads the file. A 3115-line committed artifact that no code path consumes is dead weight — it also implies to reviewers that the split-K path is being exercised, which it isn't.
Impact and fix
No runtime failure — the recipe still runs. This is a documentation-vs-code mismatch plus a large unused artifact, both of which will mislead future readers about which FlashInfer code path the benchmark actually measures. The right resolution depends on the author's intent:
- If the intent is to run with the pinned nightly + patch, uncomment lines 26-60 (or at minimum reword the header to say the swap is temporarily disabled and why).
- If the intent is to defer the swap, delete lines 7-10 of the header and remove
patches/flashinfer-cutedsl-splitk-gemm.patchfrom the tree; re-add both in the follow-up PR that enables the swap.
Since the PR is [WIP], this should be resolved before flipping out of WIP.
| - config-keys: | ||
| - minimaxm3-fp4-b300-vllm | ||
| description: | ||
| - "update vllm image" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2119 |
There was a problem hiding this comment.
🟡 The perf-changelog entry added at line 4647-4651 has two convention issues: the pr-link points to pull/2119 but this PR is #2124 (copy-paste from an unrelated PR), and the description is just "update vllm image" without the source→target image tags shown in the AGENTS.md §Updating Docker images example ("Update vLLM image from vX to vY"). It also omits the new --attention_config.indexer_kv_dtype fp8 flag and VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS=1800 env var added in the same diff, so the changelog can't be used to reconstruct what changed without cross-referencing git.
Extended reasoning...
What the bug is
The new entry in `perf-changelog.yaml` at lines 4647-4651:
```yaml
- config-keys:
- minimaxm3-fp4-b300-vllm
description: - "update vllm image"
pr-link: [WIP] Update vllm B300 Minimax M3 #2119
```
- minimaxm3-fp4-b300-vllm
has two departures from the repo's changelog conventions:
- Wrong `pr-link`. The link points to
#2119, but this is PR [WIP] Update vllm B300 Minimax M3 #2124. PR [WIP] Update vllm B300 Minimax M3 #2119 is unrelated (not in this branch's history), so this is a copy-paste from another PR/entry. - Thin `description`. `"update vllm image"` omits both the source and target image tags. The diff in `configs/nvidia-master.yaml` shows a bump from `nightly-93d8f834dd8acf33eb0e2a75b2711b628cb6e226` → `nightly-2afa3f7e950264bb179d030c23a1ed1f46558fd9`, but the changelog records neither. The same diff also adds `--attention_config.indexer_kv_dtype fp8` and `VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS=1800`, which aren't mentioned either.
Why the existing conventions matter
AGENTS.md §Updating Docker images gives the canonical example:
description: ["Update vLLM image from v0.11.2 to v0.13.0"]
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX # current PR
All recent entries in this file (#2113, #2114, #2115, #1799) follow this: their pr-link matches their own PR number, and their description names the tag/version being introduced. This one is the outlier.
Impact
This does not break the benchmark run or CI — pr-link and free-text description are audit metadata, not load-bearing for the sweep trigger (the load-bearing field is config-keys, which is correct here). Blast radius is limited to the changelog's auditability: a future reader trying to trace "which PR introduced this image bump" from the changelog alone will land on the wrong PR and won't see the tag delta or the new flags/env vars.
Step-by-step proof
git log --onelineon this branch shows commit0908dff add m3 config, which is the PR-2124 head; no reference to#2119exists in the branch history.grep -n "pull/2119" perf-changelog.yamlreturns only line 4651 — no other entry claims to come from PR [WIP] Update vllm B300 Minimax M3 #2119, ruling out "this belongs to a different logical entry".- Comparing against the entry immediately above (line 4643-4645) and the AGENTS.md example: both include image tags in
description, while this one does not.
How to fix
Update the entry to:
- config-keys:
- minimaxm3-fp4-b300-vllm
description:
- "Update vLLM image from nightly-93d8f834dd to nightly-2afa3f7e95"
- "Add --attention_config.indexer_kv_dtype fp8"
- "Set VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS=1800"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2124Noting the PR is still marked [WIP], so this is a natural cleanup before merge rather than a blocker.
| - config-keys: | ||
| - minimaxm3-fp4-b300-vllm | ||
| description: | ||
| - "update vllm image" |
There was a problem hiding this comment.
🟡 PR title and description don't satisfy the AGENTS.md bilingual requirement: title [WIP] add m3 config has no / 中文标题 suffix, and the body 中文：添加 MiniMax-M3 配置。 is Chinese-only. AGENTS.md line 7 mandates <English title> / <中文标题> and English content followed by its Chinese translation in the PR body. Worth fixing before flipping out of [WIP].
Extended reasoning...
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> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation."
This PR violates both halves of that rule:
-
Title —
[WIP] add m3 configis English-only. It needs a/ <中文标题>suffix, e.g.[WIP] add m3 config / [WIP] 添加 m3 配置. -
Description body — the current body is
中文：添加 MiniMax-M3 配置。, which is Chinese-only. The中文：prefix is telling — it labels what should be the trailing translation of a missing English original. Per the AGENTS.md ordering (English first, then Chinese), the body should read something like:Add the MiniMax-M3 NVFP4 B300 vLLM recipe and refresh the perf image. 中文：添加 MiniMax-M3 NVFP4 B300 vLLM 配方并更新 perf 镜像。
Step-by-step check against the rule:
- AGENTS.md line 7 title format requires
<English> / <中文>— the current title contains no/separator and no CJK characters, so it fails the format. - AGENTS.md line 7 body rule requires both English and Chinese, with English preceding Chinese — the current body contains only Chinese, so it fails on both count and ordering.
Impact: No runtime consequence — this is a PR-metadata convention, not a code defect. But once [WIP] is removed and the PR heads for review, reviewers will bounce it on this rule (it has been enforced on other recent PRs). Fixing it now is a trivial edit to the PR title and description on GitHub and unblocks the eventual merge.
Severity: nit — it's a WIP PR and the author is expected to polish metadata before flipping it ready-for-review. Not blocking the code content of this change.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28994920865 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=28999997980 |
jiahanc
force-pushed
the
jiahanc/m3-b300-update
branch
from
85deb99 to
0f90be4
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29017387371 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29017748440 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29026595695 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29026868938 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29027552535 |
jiahanc
force-pushed
the
jiahanc/m3-b300-update
branch
from
df94f89 to
0e761b1
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29061854550 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29062059053 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29097895328 |
jiahanc
force-pushed
the
jiahanc/m3-b300-update
branch
from
aba52c4 to
426be16
Compare
中文：添加 MiniMax-M3 配置。
中文：路由 GEMM 使用 BF16 权重。
中文：安装固定版本的 FlashInfer nightly，启用 CuTeDSL split-K 补丁，并移除 BF16 路由 GEMM 补丁。
中文：改用 FlashInfer 0.6.14 的 CUDA 13 extra，并删除单独的 CUTLASS DSL 安装步骤。
中文：改用 python3，并安装相互匹配的 FlashInfer 0.6.14 core、cubin 和 CUDA 13.0 JIT cache。
Signed-off-by: jiahanc <173873397+jiahanc@users.noreply.github.com>
中文：升级到包含上游 SM100 low-M MXFP8 split-K 的 FlashInfer nightly，删除本地 split-K 补丁，并移除显式的 TRT-LLM all-reduce backend 配置。
中文：应用 AutoTuner non-Tensor 输入保护修复。
jiahanc
force-pushed
the
jiahanc/m3-b300-update
branch
from
426be16 to
69dcebd
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29107847672 |
2 participants
No description provided.