Add DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM recipes / 添加 DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM 配方 - #2596
Add DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM recipes / 添加 DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM 配方#2596RohitNagraj wants to merge 3 commits into
Conversation
添加 DSV4 FP4 B300 Dynamo TensorRT-LLM 配置及配套配方。
|
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 关于重新运行失败任务的文档 |
在变更日志中补充拉取请求链接。
修正变更日志中的拉取请求链接。
| if [[ $FRAMEWORK == "dynamo-trt" && $MODEL_PREFIX == "dsv4" && "$MODEL_PATH" == /scratch/models/* ]]; then | ||
| SRTCTL_APPLY_ARGS+=(--no-preflight) | ||
| fi | ||
| if [[ $FRAMEWORK == "dynamo-sglang" && $MODEL_PREFIX == "dsv4" && "$MODEL_PATH" == /scratch/models/* ]]; then | ||
| SRTCTL_APPLY_ARGS+=(--no-preflight) | ||
| fi |
There was a problem hiding this comment.
🟡 Lines 257-259 (dynamo-trt) and 260-262 (dynamo-sglang) in runners/launch_b300-nv.sh are two adjacent if blocks that are identical except for the FRAMEWORK check, both appending --no-preflight for MODEL_PREFIX==dsv4 with MODEL_PATH==/scratch/models/*. This is a minor DRY duplication that could be collapsed into one condition (MODEL_PREFIX==dsv4 && (FRAMEWORK==dynamo-trt || FRAMEWORK==dynamo-sglang) && MODEL_PATH==/scratch/models/*), matching the OR pattern already used at line 254 — non-blocking cleanup.
Extended reasoning...
What the duplication is: this PR adds a new if block at runners/launch_b300-nv.sh:257-259 that appends --no-preflight to SRTCTL_APPLY_ARGS when FRAMEWORK==dynamo-trt && MODEL_PREFIX==dsv4 && MODEL_PATH==/scratch/models/*. Immediately below it, at lines 260-262, sits the pre-existing block added for the sglang launch path that does the exact same thing, differing only in the FRAMEWORK check (dynamo-sglang instead of dynamo-trt):
if [[ $FRAMEWORK == "dynamo-trt" && $MODEL_PREFIX == "dsv4" && "$MODEL_PATH" == /scratch/models/* ]]; then
SRTCTL_APPLY_ARGS+=(--no-preflight)
fi
if [[ $FRAMEWORK == "dynamo-sglang" && $MODEL_PREFIX == "dsv4" && "$MODEL_PATH" == /scratch/models/* ]]; then
SRTCTL_APPLY_ARGS+=(--no-preflight)
fiWhy it happened: the comment above these blocks (lines 249-253) already documents the rationale for bypassing the preflight check for dsv4 models staged under /scratch on the login host — this PR just copy-pasted that block for the new dynamo-trt framework rather than widening the existing condition, since nothing in the diff or the file's structure forces two separate blocks.
Why the file's own convention argues for collapsing them: line 254 already uses an OR pattern for a similar preflight bypass ($FRAMEWORK == "dynamo-vllm" && $MODEL_PREFIX == "minimaxm3" ... && ( "$CONFIG_FILE" == ... || "$CONFIG_FILE" == ... )), so collapsing the two dsv4 blocks into MODEL_PREFIX==dsv4 && (FRAMEWORK==dynamo-trt || FRAMEWORK==dynamo-sglang) && MODEL_PATH==/scratch/models/* would be consistent with how the file already expresses multi-value conditions, and is a behavior-preserving change — both blocks only ever append the identical --no-preflight flag, with no divergent side effects.
Proof by walkthrough: for FRAMEWORK=dynamo-trt, MODEL_PREFIX=dsv4, MODEL_PATH=/scratch/models/DeepSeek-V4-Pro, the first if matches and appends --no-preflight; the second if (sglang) evaluates false and is skipped. For FRAMEWORK=dynamo-sglang with the same prefix/path, the roles reverse. In both cases exactly one block fires and the net effect on SRTCTL_APPLY_ARGS is identical to what a single merged || condition would produce — confirming the two blocks are truly redundant rather than serving distinct purposes.
Impact and fix: this has no effect on correctness or CI behavior today — it's pure duplication. The risk is only that a third dsv4 framework (e.g. a future dynamo-vllm bypass) would likely be added as yet another copy-pasted block instead of extending a single shared condition. Collapsing now to if [[ $MODEL_PREFIX == "dsv4" && ( $FRAMEWORK == "dynamo-trt" || $FRAMEWORK == "dynamo-sglang" ) && "$MODEL_PATH" == /scratch/models/* ]]; then SRTCTL_APPLY_ARGS+=(--no-preflight); fi removes the near-duplicate and pre-empts that drift, at no cost to readability.
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2577 | ||
|
|
||
| - config-keys: | ||
| - dsv4-fp4-b300-dynamo-trt | ||
| description: | ||
| - "Add the DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM disaggregated configuration with STP and MTP recipe variants." | ||
| - "Run on b300 with the checkpoint at /scratch/models/DeepSeek-V4-Pro and batch_1/benchmark Slurm settings." | ||
| - "Stage the TensorRT-LLM recipes through launch_b300-nv.sh." | ||
| - "Image: nvcr.io#nvidia/tensorrt-llm/release:1.3.0rc23" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX |
There was a problem hiding this comment.
🟡 The new dsv4-fp4-b300-dynamo-trt entry in perf-changelog.yaml uses a placeholder pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX instead of the real PR number. Since this is PR #2596, it should read pull/2596 to match every other entry in the file.
Extended reasoning...
What the bug is
The newly appended dsv4-fp4-b300-dynamo-trt entry at the end of perf-changelog.yaml ends with:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXXXX is a literal, unfilled placeholder. It was never substituted with the actual pull request number.
How it manifests / code path
perf-changelog.yaml is an append-only changelog: every entry records the config keys it introduces, a human-readable description of the change, and a pr-link back to the originating PR so that a later reader (or tool) can trace why a given recipe was added or changed. Scanning the rest of the file confirms every other entry follows this convention with a concrete PR number, e.g.:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2577
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2578
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2580This PR is #2596 (per the PR metadata), so the new entry should read pull/2596.
Why nothing else catches this
There is no schema validation or lint step enforcing that pr-link resolves to a real, matching PR number — the file is plain YAML and the checklist item only requires that an entry be appended, not that its pr-link be correct. So a copy-pasted or templated placeholder like XXX silently slips through.
Impact
Low severity: this is a pure metadata/traceability defect. It does not affect benchmark execution, recipe correctness, or any runtime behavior. However, https://github.com/SemiAnalysisAI/InferenceX/pull/XXX is a dead link, and anyone (or any tooling) auditing the changelog to find the PR that introduced dsv4-fp4-b300-dynamo-trt will hit a 404 instead of the actual PR.
Step-by-step proof
- Open
perf-changelog.yamland jump to the last entry (lines 5955-5964). - Note
config-keys: [dsv4-fp4-b300-dynamo-trt]— this is the new entry added by this PR. - Note the final line:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX. - Compare against the immediately preceding entry (lines ~5940-5953), which has
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2577— a real, resolvable PR number. - Check the PR metadata for this change: it is PR Add DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM recipes / 添加 DeepSeek-V4-Pro FP4 B300 Dynamo TensorRT-LLM 配方 #2596.
- Therefore the correct value is
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2596.
Fix
Replace XXX with 2596:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2596|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31758057219 |
2 similar comments
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31758057219 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31758057219 |
Description
dsv4-fp4-b300-dynamo-trtwith 20 disaggregated B300 TensorRT-LLM recipes covering STP and MTP variants./scratch/models/DeepSeek-V4-Prousing thebatch_1partition andbenchmarkaccount.中文说明
dsv4-fp4-b300-dynamo-trt,包含 20 个面向 B300 的 TensorRT-LLM 解耦式 STP 与 MTP 配方。/scratch/models/DeepSeek-V4-Pro增加启动支持,并使用batch_1分区和benchmark账户。Related Issue
Reference / 参考:#2552
Type of Change
Checklist
perf-changelog.yamland have not edited historical entriesOWNER/MEMBER/COLLABORATOR) has commented/reuse-sweep-runon this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.