Revert #1285: original PR missing perf-changelog entry#1286
Conversation
This reverts commit 366ffbc.
|
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 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. |
There was a problem hiding this comment.
LGTM — clean revert of #1285 to allow re-landing with the required perf-changelog entry.
Extended reasoning...
Overview
This PR reverts #1285 ("Add pure TP configuration to H200 vLLM DSv4 deployment"), which had landed without the required perf-changelog.yaml entry. The diff touches three files: .github/configs/nvidia-master.yaml (removes the added pure-TP search-space rows and restores the EP+DP=8 sweep with conc-start=4, conc-end=64) and the two benchmark shell scripts dsv4_fp8_h200.sh / dsv4_fp8_h200_mtp.sh (drops the new DP_ATTENTION/EP_SIZE env-driven branching and restores the original hard-coded --enable-expert-parallel --data-parallel-size $TP invocation).
Security risks
None. This is benchmark configuration and runner shell-script content with no auth, network, or untrusted-input surface. The reverted state matches what was previously running in production prior to #1285.
Level of scrutiny
Low. This is a mechanical revert restoring a known-good prior state, and recent git history (2d8547c Revert ...#1285, 366ffbc Add pure TP...#1285) confirms the inverse-relationship. The bug hunting system found no issues, and the rationale (missing perf-changelog entry per policy) is documented in the PR description with a follow-up branch named for re-landing.
Other factors
The only timeline activity is the standard github-actions[bot] recipe-reminder comment; no outstanding human-reviewer concerns. Reverts of recently-landed config changes are exactly the kind of low-risk, self-contained change suitable for shadow approval.
…ntry (#1287) * Revert "Revert "Add pure TP configuration to H200 vLLM DSv4 deployment (#1285)"" This reverts commit 2d8547c. * Add perf-changelog entry for dsv4-fp8-h200-vllm{,-mtp} pure-TP config The original PR #1285 landed without this changelog entry; it was reverted in #1286 and is being re-landed here together with the missing entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts #1285 ("Add pure TP configuration to H200 vLLM DSv4 deployment").
The original PR landed without a corresponding
perf-changelog.yamlentry,which our policy requires for any change that alters published benchmark
configurations or sweep ranges. Reverting now and re-landing in a follow-up
PR that includes the missing changelog entry.
Re-PR: see follow-up branch
redo-1285-with-perf-changelog.🤖 Generated with Claude Code