Skip to content

Revert #1285: original PR missing perf-changelog entry#1286

Merged
cquil11 merged 1 commit intomainfrom
revert-1285-no-perf-changelog
May 6, 2026
Merged

Revert #1285: original PR missing perf-changelog entry#1286
cquil11 merged 1 commit intomainfrom
revert-1285-no-perf-changelog

Conversation

@cquil11
Copy link
Copy Markdown
Collaborator

@cquil11 cquil11 commented May 5, 2026

Reverts #1285 ("Add pure TP configuration to H200 vLLM DSv4 deployment").

The original PR landed without a corresponding perf-changelog.yaml entry,
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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

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.

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@cquil11 cquil11 merged commit ed03adc into main May 6, 2026
8 checks passed
@cquil11 cquil11 deleted the revert-1285-no-perf-changelog branch May 6, 2026 00:01
cquil11 added a commit that referenced this pull request May 6, 2026
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>
cquil11 added a commit that referenced this pull request May 6, 2026
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>
cquil11 added a commit that referenced this pull request May 6, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant