[NV] Add gb300 nvfp4 qwen3.5 MTP 8k1k recipes - #2218
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}
There was a problem hiding this comment.
Not approving — the pr-link: TBD issue flagged inline is real and will hard-fail the standard merge_with_reuse.sh canonicalization step (verified against utils/validate_perf_changelog.py and utils/prepare_perf_changelog_merge.py), so this needs a fix before merge.
Extended reasoning...
Overview
This PR adds 9 new MTP (multi-token prediction) benchmark recipe YAMLs for Qwen3.5-397B-A17B-NVFP4 on GB300, plus a corresponding configs/nvidia-master.yaml entry, a perf-changelog.yaml entry, and a new elif branch in runners/launch_gb300-nv.sh to route this model/precision/framework combination to the correct srt-slurm checkout and recipe overlay directory.
Security risks
None. This is benchmark configuration and a shell-script routing branch for an internal CI runner; no user input, auth, or secrets handling is involved.
Level of scrutiny
This falls into the repo's standard recipe-addition pattern (new YAML configs + master-config entry + changelog + runner routing), which is normally simple and mechanical. However, I independently verified the inline bug report's core claim by reading utils/validate_perf_changelog.py and utils/prepare_perf_changelog_merge.py: PR_LINK_PLACEHOLDERS is exactly {"XXX", ".../pull/XXX"}, and validate_added_pr_link/canonicalize_appended_links raise ChangelogValidationError for any pr-link value that isn't the canonical URL or one of those placeholders. The new changelog entry uses the literal string TBD, which matches neither, so the standard merge_with_reuse.sh path used by the merge-prs skill would hard-fail on this PR. That's a legitimate blocker for the standard merge tooling, even though it doesn't show up in PR-time CI checks (which don't invoke this validator).
Other factors
This is a one-line, easy fix (swap TBD for XXX or the real PR link), and the nit about mislabeling the recipes as "STP" instead of "MTP" in the changelog description is cosmetic. Neither issue affects the correctness of the recipe YAMLs, master-config entries, or runner routing logic themselves. Given the merge-blocking nature of the pr-link issue, I'm deferring rather than approving so a human/author can land the fix.
| - "Image: nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc18 with Dynamo installed at runtime (version 1.3.0.dev20260708, TCP request plane)." | ||
| - "9 STP configs: 1P1D DEP2+1D TEP16 (conc 48), 1P4D DEP2+4D TP16 (conc 24), 1P7D DEP2+7D TEP8 (conc 42), 2P15D DEP2+15D TP4 (conc 90), 3P4D DEP2+4D TEP8 (conc 192), 3P7D DEP2+7D TEP8 (conc 168), 4P1D DEP2+1D DEP16 (conc 384), 6P5D DEP2+5D TEP8 (conc 480), 9P1D DEP2+1D DEP16 (conc 1536)." | ||
| - "Runner: launch_gb300-nv.sh updated to clone NVIDIA/srt-slurm@sa-submission-q2-2026 and overlay local recipes from benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/ for the dynamo-trt+qwen3.5+fp4 path; --no-preflight added for model on compute-node /scratch." | ||
| pr-link: TBD |
There was a problem hiding this comment.
🔴 The new perf-changelog.yaml entry (line 4792) sets pr-link: TBD, but the standard merge tooling (utils/merge_with_reuse.sh → prepare_perf_changelog_merge.py canonicalize) only accepts the canonical .../pull/2218 URL or the literal XXX placeholder — TBD matches neither and will raise ChangelogValidationError, hard-failing the merge step even though PR-time checks stay green. Fix by changing it to https://github.com/SemiAnalysisAI/InferenceX/pull/2218 (or XXX, which the tooling auto-replaces at merge time).
Extended reasoning...
What the bug is: Every other entry in perf-changelog.yaml fills pr-link with either a concrete https://github.com/SemiAnalysisAI/InferenceX/pull/<n> URL or the sanctioned XXX placeholder that merge tooling auto-fills. This PR's new entry (appended at line 4792) instead uses the literal string TBD, which is not a value the changelog tooling recognizes.
Where it breaks: utils/validate_perf_changelog.py defines PR_LINK_PLACEHOLDERS = {"XXX", "https://github.com/SemiAnalysisAI/InferenceX/pull/XXX"}. Its validate_added_pr_link(link, pr_number) function checks: if a PR number is known, the link must equal the expected canonical URL or be one of PR_LINK_PLACEHOLDERS — otherwise it raises ChangelogValidationError("new PR entry must use '<url>' or an XXX placeholder; found '<link>'"). TBD satisfies neither condition.
Why PR-time checks don't catch it: The sweep-gate path (run-sweep.yml → validate_perf_changelog.py main → process_changelog.py) does not call validate_added_pr_link at all — and separately, ChangelogEntry.pr_link in matrix_logic/validation.py is an unconstrained str, so any string passes Pydantic validation. This is exactly why the issue is invisible in the PR's green checks.
Where it does fire: utils/merge_with_reuse.sh, the repository's standard reuse-assisted merge path (used by the merge-prs skill), unconditionally runs prepare_perf_changelog_merge.py canonicalize --pr-number <PR> after merging main (line 181), regardless of whether a conflict occurred. canonicalize_appended_links() calls compare_entries(base, head, pr_number), which for every appended entry calls validate_added_pr_link(entry['pr-link'], pr_number). With pr_number=2218 and link='TBD', this raises ChangelogValidationError: new PR entry must use 'https://github.com/SemiAnalysisAI/InferenceX/pull/2218' or an XXX placeholder; found 'TBD'.
Step-by-step proof:
- PR [NV] Add gb300 nvfp4 qwen3.5 MTP 8k1k recipes #2218 is merged/rebased via
merge_with_reuse.sh. - After the main-merge step, the script calls
prepare_perf_changelog_merge.py canonicalize --changelog-file perf-changelog.yaml --base-ref origin/main --pr-number 2218 --repo SemiAnalysisAI/InferenceX. canonicalize_appended_linksparses base vs. head, finds one appended entry (qwen3.5-fp4-gb300-dynamo-trt-mtp), and callsvalidate_added_pr_link('TBD', 2218).expected = 'https://github.com/SemiAnalysisAI/InferenceX/pull/2218';'TBD' not in PR_LINK_PLACEHOLDERSand'TBD' != expected→ChangelogValidationErroris raised.merge_with_reuse.shexits non-zero at that step, blocking the automated merge until someone manually fixes the changelog entry.
Impact and fix: Had the author used the sanctioned XXX placeholder, canonicalize_appended_links would have auto-rewritten it to the real /pull/2218 link and the merge would proceed silently. Using TBD instead turns what should be a no-op auto-fix into a hard failure of the standard merge tooling. The fix is a one-line change: replace TBD with either XXX or the literal https://github.com/SemiAnalysisAI/InferenceX/pull/2218.
|
|
||
| - config-keys: | ||
| - qwen3.5-fp4-gb300-dynamo-trt-mtp | ||
| description: | ||
| - "Add Qwen3.5-397B-A17B-NVFP4 GB300 disaggregated TensorRT-LLM+Dynamo MTP (3-token) benchmarks for 8k1k." | ||
| - "Image: nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc18 with Dynamo installed at runtime (version 1.3.0.dev20260708, TCP request plane)." | ||
| - "9 STP configs: 1P1D DEP2+1D TEP16 (conc 48), 1P4D DEP2+4D TP16 (conc 24), 1P7D DEP2+7D TEP8 (conc 42), 2P15D DEP2+15D TP4 (conc 90), 3P4D DEP2+4D TEP8 (conc 192), 3P7D DEP2+7D TEP8 (conc 168), 4P1D DEP2+1D DEP16 (conc 384), 6P5D DEP2+5D TEP8 (conc 480), 9P1D DEP2+1D DEP16 (conc 1536)." | ||
| - "Runner: launch_gb300-nv.sh updated to clone NVIDIA/srt-slurm@sa-submission-q2-2026 and overlay local recipes from benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/ for the dynamo-trt+qwen3.5+fp4 path; --no-preflight added for model on compute-node /scratch." | ||
| pr-link: TBD |
There was a problem hiding this comment.
🟡 The new perf-changelog.yaml entry for qwen3.5-fp4-gb300-dynamo-trt-mtp mislabels the 9 new recipes as "9 STP configs" (line ~4790), but this PR adds MTP (multi-token prediction) recipes, not STP — likely a copy-paste leftover from the sibling STP PR #2206. Should read "9 MTP configs" for accuracy.
Extended reasoning...
The new perf-changelog.yaml entry for qwen3.5-fp4-gb300-dynamo-trt-mtp is internally self-contradictory. The first description bullet correctly states "Add Qwen3.5-397B-A17B-NVFP4 GB300 disaggregated TensorRT-LLM+Dynamo MTP (3-token) benchmarks for 8k1k," but the third bullet begins "9 STP configs: 1P1D DEP2+1D TEP16 (conc 48), ...". STP (single-token prediction, i.e. no speculative decoding) and MTP (multi-token prediction, 3 speculative draft tokens) are different decoding modes, and this entry unambiguously documents the latter.
Every piece of evidence in this PR confirms these are MTP recipes: the master-config key is qwen3.5-fp4-gb300-dynamo-trt-mtp with every search-space point set to spec-decoding: "mtp"; all 9 new recipe filenames under benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/8k1k/mtp/ contain mtp3; and every recipe's speculative_config block sets decoding_type: MTP with max_draft_len: 3. There is no STP recipe anywhere in this diff.
The likely root cause is that this changelog entry was drafted by copying the changelog entry from the sibling STP recipes PR (#2206), which does add STP configs, and the "STP" wording in the count line was never updated to match the MTP content being described here — while the first bullet was correctly edited to say "MTP."
Step-by-step proof:
- Read
perf-changelog.yamllines 4784-4792 (new entry): bullet 1 says "...MTP (3-token) benchmarks..."; bullet 3 says "9 STP configs: ...". - Read
configs/nvidia-master.yaml, new keyqwen3.5-fp4-gb300-dynamo-trt-mtp: every one of the 9search-spaceentries setsspec-decoding: "mtp". - Read any of the 9 new recipe YAMLs, e.g.
ctx1dep2_gen1tep16_mtp3_c48.yaml: bothprefill.speculative_config.decoding_typeanddecode.speculative_config.decoding_typeareMTPwithmax_draft_len: 3. - Filenames themselves encode
mtp3(3-token MTP), notstp. - Therefore "STP" in bullet 3 is factually wrong for the content it describes and contradicts bullet 1 of the same entry.
This is purely a documentation string in a changelog file with no functional/runtime impact — it doesn't affect recipe execution, CI, or the master config — so it does not block merge. The fix is a one-word edit: change "9 STP configs" to "9 MTP configs" on line 4790.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29378593024 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29391306858 |
3 similar comments
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29391306858 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29391306858 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29391306858 |
|
As a PR reviewer and CODEOWNER, I have reviewed this and have:
Additional detail section:Scope. Additive-only submission: 9 new TensorRT-LLM + Dynamo disaggregated MTP recipes for Validation evidence. Run Sweep https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29391306858, triggered on head SHA
Chat template (speculative decoding item). This is a real MTP submission (
So the chat template is applied on this path; omitting the key selects Golden AL curve (agentic item). Left unchecked as not applicable. This PR is a fixed-seq-len 8k1k submission, not an agentic workload — Model architecture. No FLOP-reducing hacks. I grepped the effective diff and all 9 recipe files at head for Image / framework. The image at head — Recipe documentation item. Left unchecked as not applicable, and the upstream-PR sub-item with it. The requirement is scoped to single-node vLLM/SGLang recipes. All 9 recipes in this PR live under Patch / waiver item — full disclosure. I grepped every added line in the effective diff for
Why the code-quality item is unchecked. One concrete, fixable defect, in a line this PR adds. Other notes for the record. PR #2206 (the Qwen3.5 NVFP4 STP submission) is still OPEN / not merged; its Signed: |
❌❌❌ REJECTED ❌❌❌@Ankur-singh — two blockers: no authorized ✅ Check 0 (CODEOWNER): PASS — |
… / 修复：更正 changelog 中的 srt-slurm 引用及过期注释 The perf-changelog entry for this PR states the runner clones NVIDIA/srt-slurm@sa-submission-q2-2026, but the code this PR adds checks out v1.0.29 (runners/launch_gb300-nv.sh:235), confirmed by the sweep log (`+ git checkout v1.0.29`). perf-changelog.yaml is a permanent published record, so correct the ref rather than merge a line that misstates the PR. Also refresh the launch_gb300-nv.sh preflight comment, which still said "qwen3.5 fp4 runs on sa-submission-q2-2026" -- made stale by this same PR. Comment-and-prose only; no behaviour change. 中文：本 PR 的 perf-changelog 条目称 runner 克隆 NVIDIA/srt-slurm@sa-submission-q2-2026，但实际代码检出的是 v1.0.29（已由扫描 日志确认）。perf-changelog.yaml 为永久发布记录，故更正该引用；同时更新 launch_gb300-nv.sh 中因本 PR 而过期的注释。仅涉及注释与文案，无行为变更。
|
Withdrawing my `/reuse-sweep-run 29391306858` from 00:13 — it is no longer defensible at the current head. When I posted it, head was Two of those shared changes are on this config's execution path:
The PR's own artifacts are clean — all 9 recipes byte-identical, and the Re-authorize with an explicit run ID once a fresh sweep lands. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30506361321 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30506361321 |
|
/reuse-sweep-run 30506361321 |
|
As a PR reviewer and CODEOWNER, I have reviewed this and have:
Additional detail section:
Signed: |
|
@Klaud-Cold still waiting for the verdict |
|
Claude finished @Ankur-singh's task in 3m 34s —— View job ✅✅✅ PASS — MERGE-READY ✅✅✅@Ankur-singh — verdict rendered at head ✅ Check 0 (CODEOWNER): PASS — ✅ Check 1 (sweep on in-PR commit): PASS — run 30506361321 is a ✅ Check 2 (evals real and passing): PASS — downloaded ➖ Check 3 (recipe link): N/A — multi-node disaggregated TRT-LLM submission; the documentation requirement binds single-node vLLM/SGLang recipes only. ✅ Check 4 (reuse command): PASS — ✅ Check 5 (latest checklist): PASS — the 2026-08-06 sign-off contains all 13 items of the current ✅ Check 6 (upstream images / engine-first ordering): PASS — TRT-LLM entry on the official NGC release image (vLLM/SGLang Docker-org rule doesn't bind it); ordering satisfied by the already-merged ✅ Check 7 (MODELS.md): PASS — Qwen3.5 active scenarios at main are "Single-turn 8k1k, Agentic coding; fp8/fp4 only"; this is fp4, fixed-seq-len 8k1k, MTP. The non-MTP retirement is scoped to agentic coding and not enacted regardless. ✅ Check 8 (spec-decode chat template): PASS — no recipe sets ✅ Check 9 (no engine patches): PASS — no ➖ Check 10 (agentic golden AL): N/A — Operational note (not a gate failure)The PR is currently CONFLICTING with main ( Todo
(Note: I can't formally approve PRs — this verdict is the bot-check assessment; merging proceeds via |
✅✅✅ Verdict: PASS ✅✅✅✅ Check 0 (CODEOWNER): PASS — |
…og last / 合并 origin/main：保留两个 GB300 TRT 配置键并将 changelog 条目追加到末尾 check-changelog rejected the previous head because perf-changelog.yaml showed a deleted line (- qwen3.5-fp4-gb300-dynamo-trt-mtp): main had since gained the #2218 and #2438 entries, which this branch did not carry. Deletions are never permitted in perf-changelog.yaml. Resolution keeps main's entries intact and re-appends this PR's entry at the very end, so the diff against main is additions-only. configs/nvidia-master.yaml was an add/add conflict between this branch's qwen3.5-fp4-gb300-dynamo-trt and main's qwen3.5-fp4-gb300-dynamo-trt-mtp (#2218); both keys are kept, each with its own trailing dp-attn line.
4 participants
Add Qwen3.5-397B-A17B-NVFP4 GB300 disaggregated TensorRT-LLM + Dynamo MTP benchmarks (8k1k)
Summary
This PR adds multi-token prediction (MTP, 3 speculative tokens) benchmark recipes for Qwen3.5-397B-A17B-NVFP4 on GB300 in a disaggregated TensorRT-LLM + Dynamo setup, covering the 8k1k (ISL 8192 / OSL 1024) scenario.
Runner and master config changes follow the same pattern as PR #2206 (STP recipes), which must be merged (or the runner branch must be compatible) before CI jobs launched from this PR will succeed.
Changes
9 MTP recipe configs —
benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/8k1k/mtp/All configs use:
nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc181.3.0.dev20260708, TCP request planeqwen3.5-fp4→/scratch/models/Qwen3.5-397B-A17B-NVFP4DEP2(2 GPUs, 1 node); decode topologies varymtp3in filename)ctx1dep2_gen1tep16_mtp3_c48ctx1dep2_gen4tp16_mtp3_c4ctx1dep2_gen7tep8_mtp3_c42ctx2dep2_gen15tp4_mtp3_c90ctx3dep2_gen4tep8_mtp3_c192ctx3dep2_gen7tep8_mtp3_c168ctx4dep2_gen1dep16_mtp3_c384ctx6dep2_gen5tep8_mtp3_c480ctx9dep2_gen1dep16_mtp3_c1536runners/launch_gb300-nv.shelifbranch fordynamo-trt + qwen3.5 + fp4: clonesNVIDIA/srt-slurm@sa-submission-q2-2026and overlaysbenchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/(covers bothstp/andmtp/subdirs)--no-preflightfor this path — model lives on compute-node/scratch/NVMe, not visible from the GHA runner podconfigs/nvidia-master.yamlqwen3.5-fp4-gb300-dynamo-trt-mtp: 9 search-space points, allspec-decoding: "mtp",disagg: true,kv-p2p-transfer: nixlperf-changelog.yamlqwen3.5-fp4-gb300-dynamo-trt-mtp