Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 1 addition & 34 deletions benchmarks/single_node/fixed_seq_len/minimaxm3_fp8_b200.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,39 +22,6 @@ check_env_vars \
RANDOM_RANGE_RATIO \
RESULT_FILENAME

# The 0618 image keeps MiniMax M3 top-k indices in a persistent
# [head_kv, max_tokens, topK] buffer for CUDA graphs. Slicing that buffer to
# the actual prefill length is non-contiguous when TP leaves multiple local KV
# heads, and the MSA CSR builder rejects it. Materialize the slice until the
# image includes this fix.
python3 - <<'PYEOF' || { echo "MiniMax M3 MSA contiguity patch failed" >&2; exit 1; }
import importlib.util
import pathlib

spec = importlib.util.find_spec("vllm")
if spec is None or not spec.submodule_search_locations:
raise RuntimeError("Could not locate the installed vllm package")

target = (
pathlib.Path(next(iter(spec.submodule_search_locations)))
/ "models"
/ "minimax_m3"
/ "nvidia"
/ "sparse_attention_msa.py"
)
src = target.read_text()
old = " prefill_topk = topk[:, nd:num_tokens, :]\n"
new = " prefill_topk = topk[:, nd:num_tokens, :].contiguous()\n"

if new in src:
print(f"[minimax-m3-msa-patch] already applied: {target}")
elif src.count(old) == 1:
target.write_text(src.replace(old, new, 1))
print(f"[minimax-m3-msa-patch] patched: {target}")
else:
raise RuntimeError(f"Expected exactly one patch anchor in {target}")
PYEOF

if [[ -n "$SLURM_JOB_ID" ]]; then
echo "JOB $SLURM_JOB_ID running on $SLURMD_NODENAME"
fi
Expand Down Expand Up @@ -100,7 +67,7 @@ $PARALLEL_ARGS \
--language-model-only \
--max-cudagraph-capture-size 2048 \
--max-num-batched-tokens "$((ISL * 2 ))" \
--stream-interval 20 --no-enable-prefix-caching \
--stream-interval 32 --no-enable-prefix-caching \
--trust-remote-code > $SERVER_LOG 2>&1 &

SERVER_PID=$!
Expand Down
35 changes: 1 addition & 34 deletions benchmarks/single_node/fixed_seq_len/minimaxm3_fp8_b300.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,6 @@ check_env_vars \
RANDOM_RANGE_RATIO \
RESULT_FILENAME

# The 0618 image keeps MiniMax M3 top-k indices in a persistent
# [head_kv, max_tokens, topK] buffer for CUDA graphs. Slicing that buffer to
# the actual prefill length is non-contiguous when TP leaves multiple local KV
# heads, and the MSA CSR builder rejects it. Materialize the slice until the
# image includes this fix.
python3 - <<'PYEOF' || { echo "MiniMax M3 MSA contiguity patch failed" >&2; exit 1; }
import importlib.util
import pathlib

spec = importlib.util.find_spec("vllm")
if spec is None or not spec.submodule_search_locations:
raise RuntimeError("Could not locate the installed vllm package")

target = (
pathlib.Path(next(iter(spec.submodule_search_locations)))
/ "models"
/ "minimax_m3"
/ "nvidia"
/ "sparse_attention_msa.py"
)
src = target.read_text()
old = " prefill_topk = topk[:, nd:num_tokens, :]\n"
new = " prefill_topk = topk[:, nd:num_tokens, :].contiguous()\n"

if new in src:
print(f"[minimax-m3-msa-patch] already applied: {target}")
elif src.count(old) == 1:
target.write_text(src.replace(old, new, 1))
print(f"[minimax-m3-msa-patch] patched: {target}")
else:
raise RuntimeError(f"Expected exactly one patch anchor in {target}")
PYEOF

# `hf download` creates the target dir if missing and is itself idempotent.
# When MODEL_PATH is unset (stand-alone runs), fall back to the HF_HUB_CACHE.
# Either way, MODEL_PATH is what the server is launched with.
Expand Down Expand Up @@ -108,7 +75,7 @@ $PARALLEL_ARGS \
--language-model-only \
--max-cudagraph-capture-size 2048 \
--max-num-batched-tokens "$((ISL * 2 ))" \
--stream-interval 20 --no-enable-prefix-caching \
--stream-interval 32 --no-enable-prefix-caching \
--trust-remote-code > $SERVER_LOG 2>&1 &

SERVER_PID=$!
Expand Down
4 changes: 2 additions & 2 deletions configs/nvidia-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7345,7 +7345,7 @@ qwen3.5-fp4-b200-trt-mtp:
# 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
Comment on lines 7345 to 7351

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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:) above minimaxm3-fp8-b300-vllm
  • Lines ~7402-7410 (comment) / line 7412 (image:) above minimaxm3-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:

  1. Before this PR: image: vllm/vllm-openai:minimax-m3-0618-x86_64-cu130 on both entries, matching the comment's claim of "the dedicated ... image ... built from the m3_release branch."
  2. This PR's diff changes only the image: line to vllm/vllm-openai:nightly-4080263bb2c5d10deac17aaeb88e0823bc35bca9 for both minimaxm3-fp8-b300-vllm and minimaxm3-fp8-b200-vllm.
  3. The comment block immediately above each entry is untouched by the diff and still names the old minimax-m3-0618-x86_64-cu130 tag and the m3_release-branch rationale.
  4. 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.

Expand Down Expand Up @@ -7426,7 +7426,7 @@ minimaxm3-fp4-b300-vllm-mtp:
# /lustre/fsw/gharunners/models/MiniMax-M3-MXFP8 (pre-downloaded, see
# launch_b200-dgxc.sh).
minimaxm3-fp8-b200-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: b200-dgxc
Expand Down
7 changes: 7 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5086,6 +5086,13 @@
- "Bump SGLang container image from lmsysorg/sglang:v0.5.12-cu130 to lmsysorg/sglang:v0.5.15.post1-cu130 (https://github.com/sgl-project/sglang/releases/tag/v0.5.15.post1)"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2313

- 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
Comment on lines +5088 to +5094

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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:

  1. 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> / <中文标题>.
  2. Look at the PR metadata: title = [NV] minimax-m3-b200/b300: update fp8 STP to vLLM nightly. Split on / — the only / present is part of minimax-m3-b200/b300, a hardware pairing, not a <中文标题> suffix. There is no Chinese text anywhere in the title.
  3. Look at the PR body: it contains only ## Summary with English bullet points. Search for 中文 or any CJK characters in the body — none are present, so there's no ## 中文说明 section as required.
  4. 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.
  5. 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).


- config-keys:
- dsv4-fp4-mi355x-sglang-disagg-mtp
description:
Expand Down