Update DSV4 GB300 Dynamo vLLM Recipes#2010
Conversation
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you 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. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. 感谢你的贡献!对于 vLLM 与 SGLang,请确保你的 recipe 与官方 vLLM recipes 和/或 SGLang cookbook 保持一致 如果不一致,请先创建一个 PR,之后我们才能将你的单节点 PR 合并到 master 分支。让我们确保文档保持一流水准,使整个 ML 社区都能从你的辛勤工作中受益!谢谢 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。如果选择重新运行失败的任务,PR 作者有责任确保其最终通过。参见 GitHub 关于重新运行失败任务的文档:https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow 一般而言,PR 作者应先向相应公司的 CODEOWNERS 请求审阅并获得 PR 批准,然后再请求核心维护者审阅。 如需更多帮助,PR 作者可通过 Slack 联系核心维护者。 |
| - config-keys: | ||
| - dsv4-fp4-gb300-dynamo-vllm | ||
| description: | ||
| - "Refresh DSV4 8k/1k vLLM GB300 recipes with new w4a4 container and updated configs" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2010 |
There was a problem hiding this comment.
🟡 The new dsv4-fp4-gb300-dynamo-vllm entry in perf-changelog.yaml (line 4441) has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — a template placeholder that wasn't filled in. This should be .../pull/2010 to match the actual PR number, consistent with every other entry in the file.
Extended reasoning...
What the bug is
The new entry added at the bottom of perf-changelog.yaml for dsv4-fp4-gb300-dynamo-vllm uses a literal placeholder for its pr-link:
- config-keys:
- dsv4-fp4-gb300-dynamo-vllm
description:
- "Refresh DSV4 8k/1k vLLM GB300 recipes with new w4a4 container and updated configs"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXThe XXX is clearly a fill-in-the-blank the author forgot to substitute. Every other entry in this file uses a concrete numeric PR (e.g. pull/2001, pull/1990, pull/1978, pull/1975). This PR is #2010, so it should read https://github.com/SemiAnalysisAI/InferenceX/pull/2010.
Why existing tooling doesn't catch this on the PR
utils/validate_perf_changelog.py explicitly allows the placeholder on PR runs (via a PR_LINK_PLACEHOLDERS set) so that authors can open the PR before knowing the assigned number. That is why CI is green here.
Impact after merge
On main-branch runs, validate_added_pr_link() enforces CANONICAL_PR_LINK = r'https://github\.com/SemiAnalysisAI/InferenceX/pull/\d+' with fullmatch. XXX does not match \d+, so if this merges as-is, the next main-branch changelog validation will fail on this entry. Beyond that, the entry becomes useless as a tracking record — clicking the link goes to a 404, and the change loses its anchor to the PR that introduced it.
Step-by-step proof
- Line 4441 of the file after this PR:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX. - On PR CI,
validate_added_pr_linksees the link is inPR_LINK_PLACEHOLDERS→ passes. - After merge to
main,validate_added_pr_linkruns and executesCANONICAL_PR_LINK.fullmatch(link)with the pattern^https://github\.com/SemiAnalysisAI/InferenceX/pull/\d+$. - The value
.../pull/XXXfails\d+→fullmatchreturnsNone→ the check errors out.
Fix
Replace pull/XXX with pull/2010 on line 4441 before merging.
Severity
Nit — it does not affect runtime/benchmark behavior and is a one-character fix, but it should be corrected before merge so the changelog stays consistent and main-branch validation stays green.
No description provided.