[NV] minimax-m3-b200/b300: update fp8 STP to vLLM nightly - #2336
Conversation
…op MSA patch Single-node STP (minimaxm3-fp8-b200-vllm, minimaxm3-fp8-b300-vllm): - image -> vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9 - --stream-interval 32 - drop the sparse_attention_msa contiguity patch (shipped in the nightly image) - sweep space unchanged
|
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}
There was a problem hiding this comment.
LGTM — approving despite the two nits already flagged inline (stale comment provenance text, missing bilingual title/description); neither affects correctness of the recipe change.
Extended reasoning...
Overview
This PR touches only benchmark recipe files for MiniMax-M3 MXFP8 STP on B200/B300: two shell scripts (minimaxm3_fp8_b200.sh, minimaxm3_fp8_b300.sh), configs/nvidia-master.yaml, and perf-changelog.yaml. The change bumps the vLLM image to a nightly build, raises --stream-interval from 20 to 32, and removes an inline Python monkey-patch that materialized a non-contiguous tensor slice in vLLM'''s MiniMax-M3 sparse-attention MSA path — a workaround the PR description says is now fixed upstream in the nightly image. The removed patch block is identical in both scripts and its removal is clean (no dangling references elsewhere in the scripts). The perf-changelog.yaml entry accurately describes all three changes and links back to this PR.
Security risks
None. No auth, crypto, permissions, network-facing, or user-input-handling code is touched. The removed code was a local file patch applied to the container'''s installed vllm package at container-start time, not anything user-reachable.
Level of scrutiny
Low-to-moderate. This is a benchmark-recipe change in a config-driven sweep system, not production-critical application code. The image bump and flag change are mechanical, and the patch removal is a straightforward revert of a previously-added workaround now claimed to be fixed upstream — consistent with the stated rationale in both the comment being removed and the PR description. The two issues found (stale rationale comment left in nvidia-master.yaml describing the old 0618 image/m3_release branch, and a missing bilingual PR title/description per AGENTS.md) are documentation/process nits with zero runtime or CI impact.
Other factors
The bug-hunting pass also checked for and ruled out a duplicate framing of the same stale-comment issue, so there'''s nothing additional beyond what'''s already inline. No outstanding reviewer comments block this PR — the only timeline activity is the standard automated recipe-reminder comment. Given the narrow, mechanical scope and absence of any functional defect, I'''m comfortable approving with the two nits left as inline notes for the author to clean up.
| # sparse/index cache alignment). Weights are NOT SRE-staged; b300 falls back | ||
| # to writable /data/models (see launch_b300-nv.sh MODEL_PATH split). | ||
| minimaxm3-fp8-b300-vllm: | ||
| image: vllm/vllm-openai:minimax-m3-0618-x86_64-cu130 | ||
| image: vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9 | ||
| model: MiniMaxAI/MiniMax-M3-MXFP8 | ||
| model-prefix: minimaxm3 | ||
| runner: b300 |
There was a problem hiding this comment.
🟡 The block comments above both minimaxm3-fp8-b300-vllm and minimaxm3-fp8-b200-vllm still say the image is a dedicated build from the m3_release branch (vllm-project/vllm#45381) because M3 hasn't shipped in a stable vLLM release, but this PR changes both image: fields to a nightly build. The rationale/provenance sentence is now stale in both places and should be updated (or removed) to reflect that a nightly image is used instead.
Extended reasoning...
This PR bumps the image: field for both the B300 and B200 MiniMax-M3 FP8 STP configs in configs/nvidia-master.yaml from vllm/vllm-openai:minimax-m3-0618-x86_64-cu130 to vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9, but it leaves the explanatory block comments directly above each entry untouched. Both comments read (identically, word for word): "M3 support has not shipped in a stable vLLM release; the dedicated vllm/vllm-openai:minimax-m3-0618-x86_64-cu130 image is built from the m3_release branch (vllm-project/vllm#45381)." That sentence now names a container tag the config no longer references and asserts a provenance story ("dedicated ... image ... built from the m3_release branch") that no longer applies to a generic nightly build.
The two locations are:
- Lines ~7322-7329 (comment) / line 7331 (
image:) aboveminimaxm3-fp8-b300-vllm - Lines ~7402-7410 (comment) / line 7412 (
image:) aboveminimaxm3-fp8-b200-vllm
Nothing in the PR or the surrounding YAML would catch this: comments are not validated against the fields they describe, there is no schema or lint rule tying image: values to comment text, and the diff hunk for the image-line change does not include the comment block above it, so a reviewer skimming the diff would only see the new image: value, not the stale sentence sitting just outside the hunk context.
The impact is documentation-only — nothing breaks at runtime, CI, or in the benchmark sweep, since the comment is never parsed or executed. The risk is purely that a future engineer reading this config (e.g., to decide whether to bump the image again, or to understand why a "dedicated" image was pinned) will be misled into thinking the current nightly image is still the special m3_release-branch build tied to vllm-project/vllm#45381, when it is actually an unrelated nightly snapshot.
Step-by-step proof:
- Before this PR:
image: vllm/vllm-openai:minimax-m3-0618-x86_64-cu130on both entries, matching the comment's claim of "the dedicated ... image ... built from the m3_release branch." - This PR's diff changes only the
image:line tovllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9for bothminimaxm3-fp8-b300-vllmandminimaxm3-fp8-b200-vllm. - The comment block immediately above each entry is untouched by the diff and still names the old
minimax-m3-0618-x86_64-cu130tag and the m3_release-branch rationale. - Reading the file post-merge, a reader sees
image: vllm/vllm-openai:nightly-...directly under a comment that describes a completely different image and its provenance — a direct contradiction between comment and code.
Fix: update (or trim) the comment in both places to reflect that a nightly vLLM build is now used, e.g., replace the "dedicated ... m3_release branch" sentence with something like "a nightly vLLM build is used until M3 support lands in a stable release," or simply drop the specific tag/PR reference since it no longer applies.
|
|
||
| - config-keys: | ||
| - minimaxm3-fp8-b200-vllm | ||
| - minimaxm3-fp8-b300-vllm | ||
| description: | ||
| - "Bump image to vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9; set --stream-interval 32; drop the sparse_attention_msa contiguity patch (shipped in the nightly image)" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2336 |
There was a problem hiding this comment.
🟡 This PR's title and description are English-only, but AGENTS.md mandates bilingual PR titles/descriptions (title format <English> / <中文标题>, plus a ## 中文说明 section in the body). Please add the Chinese title suffix and a Chinese summary section to comply with the repo's contribution guidelines.
Extended reasoning...
What the rule requires: AGENTS.md line 7 states explicitly:
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 (e.g. a## 中文说明section mirroring the summary...). This applies to every PR and every issue.
The only carve-outs are the CODEOWNER sign-off template (which must stay English-verbatim so the verifier bot can match its exact trigger phrase) and bot-generated comments. Neither exception covers the PR author's own title or Summary section.
How this PR violates it: The PR title is [NV] minimax-m3-b200/b300: update fp8 STP to vLLM nightly — English only, with no / <中文标题> suffix. The description consists solely of an English ## Summary section (image bump, --stream-interval 32, dropping the MSA contiguity patch); there is no ## 中文说明 section mirroring it.
Why this isn't caught automatically: Nothing in this PR's diff (the benchmark scripts, configs/nvidia-master.yaml, or perf-changelog.yaml) enforces or validates the bilingual title/description requirement — it's a human/PR-metadata convention rather than something checked by the changelog validator or the sign-off bot. The sign-off bot only checks for the literal English phrase As a PR reviewer and CODEOWNER, I have reviewed this and have in a reviewer comment; it has no logic inspecting the PR title or Summary body for Chinese content. So a purely English title/description sails through CI untouched, and the requirement can only be caught by review (human or otherwise) against AGENTS.md.
Step-by-step proof:
- Open AGENTS.md and read line 7 — the bilingual title/description mandate applies to "every PR and every issue," with the title format
<English title> / <中文标题>. - Look at the PR metadata: title =
[NV] minimax-m3-b200/b300: update fp8 STP to vLLM nightly. Split on/— the only/present is part ofminimax-m3-b200/b300, a hardware pairing, not a<中文标题>suffix. There is no Chinese text anywhere in the title. - Look at the PR body: it contains only
## Summarywith English bullet points. Search for中文or any CJK characters in the body — none are present, so there's no## 中文说明section as required. - Confirm no exception applies: this is not the CODEOWNER sign-off template (those are the two
github-actions[bot]timeline comments, not the PR title/description) and not a bot-generated comment — it's the author's own title and Summary. - Conclusion: the PR violates the explicit, repo-mandated bilingual requirement.
Impact and fix: This is a process/documentation-convention gap, not a functional defect — nothing in the benchmark scripts, config, or changelog entry is incorrect, and no CI job or benchmark run will fail because of it. The fix is straightforward: rename the title to [NV] minimax-m3-b200/b300: update fp8 STP to vLLM nightly / <中文标题> and append a ## 中文说明 section to the body mirroring the English Summary (bump image, set --stream-interval 32, drop the MSA patch since it's fixed upstream), following the translation quality bar described elsewhere in AGENTS.md (natural technical Chinese, keeping model names/flags/SKUs in English).
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30208026609 |
2 similar comments
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30208026609 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30208026609 |
|
/reuse-sweep-run |
2 participants
Summary
Single-node STP (non-MTP) MiniMax-M3 MXFP8 on B200/B300 (
minimaxm3-fp8-b200-vllm,minimaxm3-fp8-b300-vllm):vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9--stream-interval 32sparse_attention_msacontiguity patch — the fix ships in the nightly image