Skip to content

[NV] Add dynamo trtllm gb300 qwen3.5 nvfp4 recipes for 8k1k and 1k1k#2206

Open
richardhuo-nv wants to merge 4 commits into
mainfrom
dynamo-trtllm-gb300-qwen3.5-nvfp4
Open

[NV] Add dynamo trtllm gb300 qwen3.5 nvfp4 recipes for 8k1k and 1k1k#2206
richardhuo-nv wants to merge 4 commits into
mainfrom
dynamo-trtllm-gb300-qwen3.5-nvfp4

Conversation

@richardhuo-nv

Copy link
Copy Markdown
Collaborator

Summary

Add Qwen3.5-397B-A17B-NVFP4 GB300 disaggregated TensorRT-LLM + Dynamo multinode benchmarks (STP, no MTP) for both 1k1k and 8k1k scenarios.

  • 18 srt-slurm recipe configs checked into benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/ (9 per scenario), sourced from validated experiment runs
  • Master config entry qwen3.5-fp4-gb300-dynamo-trt covering the full topology sweep across both ISL/OSL scenarios
  • Runner launch_gb300-nv.sh extended to clone NVIDIA/srt-slurm@sa-submission-q2-2026 and overlay local TRT-LLM recipes for the dynamo-trt + qwen3.5 + fp4 path
  • Perf-changelog entry to trigger the full GB300 sweep

Config details

Image: nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc18
Dynamo: installed at runtime (version: 1.3.0.dev20260708, TCP request plane)
Model path: /scratch/models/Qwen3.5-397B-A17B-NVFP4 (node-local NVMe)
Model alias: qwen3.5-fp4 (matches model.path in all recipes)

All prefill workers use TP2/EP2 with attention DP (DEP2, 2 GPUs/worker). Decode topology varies per concurrency point:

1k1k (ISL=1024, OSL=1024) — 9 configs

Config Prefill Decode Conc
ctx1dep2_gen17dep4_mtp0_c68 1×DEP2 17×TP4/EP4 68
ctx1dep2_gen17dep4_mtp0_c136 1×DEP2 17×TP4/EP1 136
ctx1dep2_gen17dep4_mtp0_c272 1×DEP2 17×TP4/EP1 272
ctx1dep2_gen4dep16_mtp0_c16 1×DEP2 4×TP16/EP16 16
ctx1dep2_gen5dep8_mtp0_c640 1×DEP2 5×TP8/EP8 640
ctx1dep2_gen1dep16_mtp0_c1280 1×DEP2 1×DEP16 1280
ctx2dep2_gen3dep16_mtp0_c1536 2×DEP2 3×DEP16 1536
ctx5dep2_gen3dep16_mtp0_c7680 5×DEP2 3×DEP16 7680
ctx5dep2_gen7dep4_mtp0_c17920 5×DEP2 7×DEP4 17920

8k1k (ISL=8192, OSL=1024) — 9 configs

Config Prefill Decode Conc
ctx1dep2_gen13tp4_mtp0_c78 1×DEP2 13×TP4/EP1 78
ctx1dep2_gen4tep16_mtp0_c24 1×DEP2 4×TP16/EP16 24
ctx1dep2_gen8tep8_mtp0_c48 1×DEP2 8×TP8/EP8 48
ctx2dep2_gen15tp4_mtp0_c180 2×DEP2 15×TP4/EP1 180
ctx2dep2_gen9tp4_mtp0_c216 2×DEP2 9×TP4/EP1 216
ctx3dep2_gen1dep4_mtp0_c1536 3×DEP2 1×DEP4 1536
ctx7dep2_gen12tep4_mtp0_c1152 7×DEP2 12×TP4/EP4 1152
ctx8dep2_gen9tep4_mtp0_c1728 8×DEP2 9×TP4/EP4 1728
ctx10dep2_gen3dep8_mtp0_c4608 10×DEP2 3×DEP8 4608

Files changed

  • benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/1k1k/stp/ — 9 new recipe YAMLs
  • benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg/8k1k/stp/ — 9 new recipe YAMLs
  • .github/configs/nvidia-master.yaml — new qwen3.5-fp4-gb300-dynamo-trt entry (18 search-space points)
  • runners/launch_gb300-nv.sh — new elif branch for dynamo-trt + qwen3.5 + fp4 to overlay local recipes
  • perf-changelog.yaml — new entry to trigger the GB300 sweep

@claude claude Bot left a comment

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.

Additional findings (outside current diff — PR may have been updated during review):

  • 🔴 perf-changelog.yaml:4355 — The new entry's pr-link is https://github.com/SemiAnalysisAI/InferenceX/pull/{TBD}, but the changelog merge tooling only accepts a real pull/<PR> link or the literal placeholder XXX; {TBD} matches neither. This won't fail the PR-time CI gate, but it will raise a ChangelogValidationError when utils/merge_with_reuse.sh runs prepare_perf_changelog_merge.py canonicalize at merge time, blocking the standard merge path until fixed. Fix: replace {TBD} with XXX (or the literal pull/2206).

    Extended reasoning...

    The newly appended perf-changelog.yaml entry (line 4355) sets:

    pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/{TBD}
    

    The accepted values for an appended entry's pr-link are the canonical pull/<PR-number> URL, or one of the literal placeholders in PR_LINK_PLACEHOLDERS ('XXX' or 'https://github.com/SemiAnalysisAI/InferenceX/pull/XXX', defined in utils/validate_perf_changelog.py lines 24-27). The string {TBD} is neither of these, so it will be rejected by validate_added_pr_link().

    Where it actually fires: it's worth correcting where this gets caught, since it's not where it would naively be assumed. The PR-time CI gate (run-sweep.yml's "Validate perf-changelog matrix" step) invokes validate_perf_changelog.py's main(), which only calls validate_matrix_compatible_changevalidate_generated_config (process_changelog.py). That code path never inspects pr-link at all — ChangelogEntry.pr_link (utils/matrix_logic/validation.py:497) is a plain str field with no format validator — so {TBD} sails through the PR CI check without complaint.

    The rejection instead happens at merge time. utils/merge_with_reuse.sh (the documented, required merge entrypoint per README.md and the merge-prs skill) unconditionally runs prepare_perf_changelog_merge.py canonicalize --pr-number <PR> as part of the merge flow. That script calls canonicalize_appended_links()compare_entries()validate_added_pr_link(link, pr_number). With pr_number supplied, the expected value is the real pull/2206 link, and the only alternative accepted is the XXX placeholder (meant to be auto-substituted with the real PR number during canonicalization). {TBD} matches neither, so validate_added_pr_link raises ChangelogValidationError('new PR entry must use ... or an XXX placeholder; found ...{TBD}'), and the merge script (running under set -euo pipefail) hard-fails.

    Step-by-step proof:

    1. PR #2206 is opened with perf-changelog.yaml's new entry containing pr-link: .../pull/{TBD}.
    2. The PR-time CI check (validate_perf_changelog.py main()) runs and passes, since it never looks at pr-link.
    3. A maintainer runs the standard merge flow: utils/merge_with_reuse.sh <PR>.
    4. That script invokes prepare_perf_changelog_merge.py canonicalize --pr-number 2206.
    5. canonicalize_appended_links iterates the appended changelog entries, calling compare_entries(base, head, 2206) for each.
    6. For this entry, link = 'https://github.com/SemiAnalysisAI/InferenceX/pull/{TBD}'. It is compared against the expected canonical link (pull/2206) and against PR_LINK_PLACEHOLDERS = {'XXX', '.../pull/XXX'}. It matches neither.
    7. validate_added_pr_link raises ChangelogValidationError, aborting the merge script before the rest of the PRs in the batch (if any) are processed.

    Impact: the PR looks green in CI but cannot be merged via the standard/enforced merge tooling until the placeholder is fixed — someone has to notice the failure, edit perf-changelog.yaml, and re-push before merge can proceed.

    Fix: replace {TBD} with the XXX placeholder (matching the convention used by essentially every other entry in this file, which lets canonicalization substitute the real PR number automatically) or with the literal pull/2206 link.

  • 🔴 runners/launch_gb300-nv.sh:205-211 — In the new dynamo-trt + qwen3.5 + fp4 branch, the runner checks out v1.0.29 from NVIDIA/srt-slurm, but every other sibling branch in this same if/elif chain (including the closest analog, dsv4/dynamo-trt directly above) checks out sa-submission-q2-2026 — and this PR's own perf-changelog entry states the runner clones @sa-submission-q2-2026. If unintentional, the recipes overlaid into this checkout (authored against the submission branch's srtctl schema/layout) will likely fail to apply against the public v1.0.29 tag, breaking the sweep at launch.

    Extended reasoning...

    The bug: runners/launch_gb300-nv.sh:208 (the new elif for FRAMEWORK==dynamo-trt && MODEL_PREFIX==qwen3.5 && PRECISION==fp4) does git checkout v1.0.29 after cloning NVIDIA/srt-slurm. Every other branch of this same if/elif chain that touches dynamo-trt or dynamo-sglang — the dsv4/dynamo-trt branch directly above it (line 204), the glm5/qwen3.5 dynamo-sglang branches, and the final else fallback — all check out sa-submission-q2-2026. This PR's own perf-changelog.yaml entry, added in the very same diff, spells this out explicitly: "Runner: launch_gb300-nv.sh updated to clone NVIDIA/srt-slurm@sa-submission-q2-2026 ... for the dynamo-trt+qwen3.5+fp4 path." So the code, the changelog, and every sibling code path in the file all point to sa-submission-q2-2026, and only this one new branch diverges to a different ref.

    Why this looks like a real bug, not a stale description: normally a description/code mismatch is more likely to be an imprecise PR write-up than a code defect (per the "nit by default" guidance). That reasoning doesn't hold as cleanly here, because the "description" in question isn't a vague high-level PR summary — it's a changelog entry added in the same commit that describes this exact new branch's behavior, and it agrees with five other code paths written in the same file, in the same PR history, using the same clone-and-checkout idiom. The odds that all of those independently converged on the wrong branch name, while only the new elif happens to be correct, are low. The much more parsimonious explanation is a copy/paste or typo slip when this branch was authored (e.g., pinning a tag while drafting/testing locally, then forgetting to switch back to the shared submission branch before opening the PR).

    Code path / mechanism: launch_gb300-nv.sh clones NVIDIA/srt-slurm fresh into $SRT_REPO_DIR, checks out a ref, and then this new branch does mkdir -p recipes/trtllm/qwen3.5/gb300-fp4/disagg followed by cp -rT .../benchmarks/multi_node/srt-slurm-recipes/trtllm/qwen3.5/gb300-fp4/disagg recipes/trtllm/qwen3.5/gb300-fp4/disagg, overlaying the 18 new recipe YAMLs into the checked-out repo. srtctl apply is then run against that recipe tree. The recipes carry provenance headers (# Source: /lustre/.../configs-variant1/...yaml) showing they were authored/validated against a specific run of srtctl/TensorRT-LLM — but that provenance says nothing about which srt-slurm git ref was used, so it doesn't establish that v1.0.29 (a public upstream tag) is compatible, and it cuts equally in favor of sa-submission-q2-2026 since that's the ref the PR author says was used for the whole PR.

    Why nothing in the existing code prevents this: the if/elif structure has no shared "default srt-slurm ref" variable — each branch independently hardcodes its own git checkout <ref>, so there is no single point of truth that would catch a rogue ref, and nothing in srtctl apply validates ahead of time that the recipe's on-disk schema matches the checked-out srt-slurm version until the actual apply/parse step runs.

    Impact if this is indeed a mistake: srtctl apply would attempt to parse/apply the 18 new recipe YAMLs (each with a trtllm_config, cache_transceiver_config, moe_config, dynamo: block, etc.) against whatever schema/CLI v1.0.29 ships, which may differ from the submission branch's schema the recipes were actually written for. In the best case this fails loudly at srtctl apply time (unknown field / validation error) and the whole GB300 qwen3.5 sweep — the entire point of this PR — never launches. In a worse case, if v1.0.29's schema happens to overlap partially, it could silently apply a subset of settings and produce misleading benchmark numbers instead of a clean failure.

    Step-by-step proof of the inconsistency (this part is fully verifiable from the diff alone, no external repo access needed):

    1. Diff line ~204 (dsv4/dynamo-trt, the closest analog — same FRAMEWORK): git checkout sa-submission-q2-2026.
    2. Diff line ~208 (new qwen3.5/dynamo-trt/fp4 branch): git checkout v1.0.29.
    3. Diff to perf-changelog.yaml, added in this same PR, describing this same new branch: "Runner: launch_gb300-nv.sh updated to clone NVIDIA/srt-slurm@sa-submission-q2-2026 and overlay local recipes ... for the dynamo-trt+qwen3.5+fp4 path."
    4. The else fallback (line ~215, catches every framework/model combo not otherwise matched) also uses sa-submission-q2-2026.
    5. Four independent sources within this one file/PR (three sibling code branches + the fallback + the changelog prose) agree on sa-submission-q2-2026; only the new branch's actual executed git checkout disagrees.

    Suggested fix: change line 208 from git checkout v1.0.29 to git checkout sa-submission-q2-2026, matching the sibling dynamo-trt branch and the PR's own changelog — unless the author confirms v1.0.29 was a deliberate, tested pin, in which case the changelog line should be corrected instead so the two aren't contradictory.

    One counterpoint raised during review deserves acknowledgment: it's possible v1.0.29 is an intentional, deliberate pin to a stable release tag (arguably more reproducible than tracking a moving branch), and the changelog prose is simply reused boilerplate that wasn't updated for this specific branch. I can't rule this out without access to the upstream NVIDIA/srt-slurm repo to check whether v1.0.29 actually supports the TensorRT-LLM disagg recipe schema these 18 YAMLs use. That uncertainty is exactly why this should be flagged for the author to confirm before merge, rather than silently waved through — but the concrete failure mode (sweep launch fails) if it's wrong, combined with the accumulated weight of evidence (5 independent agreements vs. 1 outlier), pushes this from a nice-to-have nit to something the author should explicitly resolve before merging.

@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@github-actions

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant