[AgentX] Update vLLM DeepSeek-V4 B300 aggregate / 更新 vLLM DeepSeek-V4 B300 聚合配置#2241
Conversation
Signed-off-by: Jeff Ma <jeffjma@umich.edu>
Signed-off-by: Jeff Ma <jeffjma@umich.edu>
Signed-off-by: Jeff Ma <jeffjma@umich.edu>
ivanium
added
NVIDIA
full-sweep-enabled
agentx
|
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 关于重新运行失败任务的文档 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LjHXfE1FRN9c5XQqgeTdez
|
Claude finished @ivanium's task in 3m 33s —— View job Review complete
SummaryNo blocking issues found. One 🟡 warning posted inline: the new Everything else checks out: master config + perf-changelog updated together, new changelog entry appended at the end of the file, image is a public Docker Hub reference, |
Restore upstream lines whitespace-for-whitespace and append the B300 entry so the changelog diff contains no deletions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LjHXfE1FRN9c5XQqgeTdez
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
perf-changelog.yaml:4865-4869— This PR strips trailing whitespace from two pre-existing, unrelated lines in perf-changelog.yaml: the blank separator line after the pull/2222 entry, and the end of the pull/2201 pr-link line. AGENTS.md explicitly documents that CI depends on exact whitespace in this file (including trailing spaces on separator lines) and that altering it breaks CI, so these two incidental whitespace changes should be reverted.Extended reasoning...
AGENTS.md (line 176) states verbatim: "Never delete or modify whitespace in
perf-changelog.yaml- CI depends on exact whitespace (including trailing spaces on blank separator lines). Altering it breaks CI." This is a repo-specific, machine-checkable rule (not a style preference), and the diff violates it twice in code that has nothing to do with the PR's actual purpose (adding a new changelog entry for the dsv4-fp4-b300-vllm-agentic update).Looking at the diff hunk for
perf-changelog.yaml:- Around line 4809-4813: the blank line immediately after
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2222previously contained two trailing spaces (-in the diff, i.e. a line consisting of two spaces). The PR changes this to a truly empty line (+with nothing after it). - Around line 4868: the line
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2201previously ended with two trailing spaces. The PR strips them, leavingpr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2201with no trailing whitespace.
Neither of these lines is part of the new entry being appended for this PR (the new dsv4-fp4-b300-vllm-agentic block is appended afterward, starting at line 4869 in the new file). They are pre-existing lines from prior, unrelated changelog entries (PR #2222 and PR #2201) that this diff incidentally touches and reformats, most likely because a local editor or formatter auto-stripped trailing whitespace when the file was opened/saved.
Nothing in the existing codebase or CI configuration prevents this: there is no lint rule, pre-commit hook, or editor config enforced in this repo that would catch or block trailing-whitespace stripping in this file — the only protection is the documented convention in AGENTS.md, which this PR silently violates. Since the repository's own documentation asserts a concrete, verifiable consequence (CI depends on this exact whitespace and will break if it's altered), this is not speculative: it's a stated fact about how the changelog is likely consumed/validated downstream.
Step-by-step proof from the diff:
- Old line (context in diff, unified diff shows removed line with trailing spaces):
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2222followed by a separator line containing exactly two space characters and nothing else. - New line: the same separator line now contains zero characters (fully empty).
- Old line:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2201(two trailing spaces after2201). - New line:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2201(no trailing spaces). - Per AGENTS.md line 176, both of these edits are exactly the kind of whitespace alteration that is prohibited and stated to break CI.
Fix: restore the original two trailing spaces on both the blank separator line after the pull/2222 pr-link and at the end of the pull/2201 pr-link line, leaving only the newly appended entry (for this PR) as the actual content change to perf-changelog.yaml.
- Around line 4809-4813: the blank line immediately after
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29486959583 |
|
vLLM recipe PR: vllm-project/recipes#645 |
|
/reuse-sweep-run |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29486959583 |
cquil11
left a comment
cquil11
left a comment
There was a problem hiding this comment.
As a PR reviewer and CODEOWNER, I have reviewed this and have:
- Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
- Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
- Verified that this PR has passed PR validation: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29486959583
- Verified that this PR passes evals: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29486959583
- Verified that the speculative-decoding chat-template requirement is not applicable because this is the non-MTP/non-speculative configuration.
- Verified that the agentic speculative-decoding synthetic-acceptance requirement is not applicable because this is the non-MTP/non-speculative configuration.
- Verified that the model architecture is not changed with benchmark hacks or reductions in model-architecture FLOPs.
- Verified that the vLLM submission uses the upstream
vllm/vllm-openai:nightly-dev-x86_64-cu13.0.1-904e4ecimage. - Verified that the upstream in-tree vLLM submission requirement precedes additional frameworks; this PR adds only a vLLM configuration.
- Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or SGLang cookbook.
- Linked a corresponding merged upstream recipe PR or an already-published recipe page.
- Verified that this PR does not patch the inference engine or serving stack; the pinned image runs as shipped.
- If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.
Additional detail section:
- PR validation completed successfully on retry attempt 2: 36 jobs succeeded, 15 were skipped, and none failed.
- The requested changelog check passed: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29486959583/job/87713390147
- The branch includes the latest
main, the append-onlyperf-changelog.yamlconflict was resolved, and local Bash syntax plus all 221 matrix tests passed. - No merged upstream vLLM recipe PR or published recipe link was provided or found, so the two recipe-documentation boxes are intentionally left unchecked rather than asserting unsupported evidence. The repository administrator explicitly authorized an admin merge despite this unmet checklist item.
Signed: cquil11
3 participants
No description provided.