[chore] Bump dsv4-fp4 vLLM image to v0.25.0 on B200 and B300#2169
[chore] Bump dsv4-fp4 vLLM image to v0.25.0 on B200 and B300#2169xinli-sw wants to merge 3 commits into
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 关于重新运行失败任务的文档 |
|
|
||
| dsv4-fp4-b200-vllm: | ||
| image: vllm/vllm-openai:nightly-3f0a91bb96f8d72e0498b95c166e817deae14d62 | ||
| image: vllm/vllm-openai:v0.24.0 |
There was a problem hiding this comment.
🔴 This PR bumps the vLLM image tag on four dsv4-fp4 configs but does not append a corresponding perf-changelog.yaml entry. Per AGENTS.md's 'Updating Docker images' section, this entry is "required — triggers benchmarks", so without it the sweep won't validate v0.24.0 on these configs and the bump ships un-benchmarked. Please add a perf-changelog.yaml entry listing dsv4-fp4-b200-vllm, dsv4-fp4-b200-vllm-mtp, dsv4-fp4-b300-vllm, dsv4-fp4-b300-vllm-mtp before merging.
Extended reasoning...
What the bug is. This PR bumps the image: field on four configs in configs/nvidia-master.yaml from various older tags to vllm/vllm-openai:v0.24.0 — dsv4-fp4-b200-vllm (line 1742), dsv4-fp4-b200-vllm-mtp (line 1830), dsv4-fp4-b300-vllm (line 3184), and dsv4-fp4-b300-vllm-mtp (line 3279). The changed-files list on this PR is only configs/nvidia-master.yaml — no companion entry was appended to perf-changelog.yaml.\n\nWhy this is required. AGENTS.md § 'Updating Docker images' is explicit: "Update the image tag in the relevant configs/*-master.yaml and/or benchmarks/*.sh, update any related env vars / config params, and append a perf-changelog.yaml entry (required — triggers benchmarks)". The parenthetical spells out the consequence: perf-changelog.yaml is what selects which config-keys the sweep actually runs. Elsewhere in AGENTS.md it's described as 'the benchmark trigger log'. Without an entry naming these four config-keys, the full-sweep does not pick them up for validation against the new image.\n\nWhy existing code doesn't prevent it. There is no CI check that cross-references image-tag diffs in configs/*-master.yaml against perf-changelog.yaml additions — the convention is documented but not enforced. A PR that only touches the image tag will pass lint and typecheck; the sweep simply won't cover the changed configs.\n\nImpact. The stated purpose of this PR (per its title, '[chore] Bump dsv4-fp4 vLLM image to v0.24.0 on B200 and B300') is to validate v0.24.0 on the four dsv4 FP4 vLLM configs. Merging without the changelog entry means the image change ships to main without any of those four configs being benchmarked against v0.24.0. Any regression in v0.24.0 (accuracy, throughput, or an outright crash on DeepSeek-V4-Pro) would not be caught until a downstream benchmark or a real user runs it.\n\nHow to fix. Append an entry to perf-changelog.yaml naming all four bumped config-keys. Following the format used by every recent analogous PR (#2073 for kimik2.5-int4-b200-vllm v0.24.0, #2070 for kimik2.5-int4-b300-vllm v0.24.0, #2074, #2077 — each of these mirrors this PR's shape and each appended such an entry):\n\nyaml\n- config-keys:\n - dsv4-fp4-b200-vllm\n - dsv4-fp4-b200-vllm-mtp\n - dsv4-fp4-b300-vllm\n - dsv4-fp4-b300-vllm-mtp\n description:\n - "Bump vLLM image to v0.24.0"\n pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2169\n\n\nStep-by-step proof this manifests.\n1. PR author changes image: on dsv4-fp4-b200-vllm from a nightly tag to v0.24.0 and does the same for the three siblings.\n2. PR is labeled full-sweep-fail-fast per the recipe-reminder bot.\n3. The sweep launcher reads perf-changelog.yaml to determine which config-keys to run in the sweep for this PR.\n4. Grep of the current perf-changelog.yaml for '2169' or 'v0.24.0' on any of dsv4-fp4-b200-vllm/-mtp/dsv4-fp4-b300-vllm/-mtp: no match — the four bumped configs are not in the trigger set for this PR.\n5. Sweep completes green because it did not run any of the four configs whose image actually changed. The bump is merged as 'validated' when in fact no dsv4-fp4 vLLM benchmark exercised v0.24.0.\n\nThis is a required convention that gates the actual validation the PR is intended to perform, so it should be resolved before merge.
35f8ba8 to
f4c5583
Compare
f4c5583 to
df93b60
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29138257980 |
No description provided.