-
Notifications
You must be signed in to change notification settings - Fork 155
retry sglang b300 #1171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
retry sglang b300 #1171
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The new changelog entry at lines 1859-1864 is byte-identical to the existing entry at lines 1782-1787 (whose pr-link was just corrected to #1132 in this same PR), but attributes the same 'Restore the recipe-per-CONC split' description to PR #1158 — which was actually titled 'fix sgl b200/b300 dpsk-v4 script' and did not introduce that work. Per the precedent set by retrigger PRs #1148 (line 1827) and #1160 (line 1835), this PR (#1171, 'retry sglang b300') should use its own pr-link with a 'Retrigger dsv4-fp4-b300-sglang' description rather than copy-pasting from #1132.
Extended reasoning...
What the bug is
After this PR,
perf-changelog.yamlcontains two entries fordsv4-fp4-b300-sglangwhosedescriptionarrays are byte-identical:pr-link: .../pull/1132— the description was just corrected from fix sgl b200/b300 dpsk-v4 script #1158 to [NVIDIA] chore: B300 single node DeepSeek v4 SGLang #1132 in this PR (the existing recipe-per-CONC split work).pr-link: .../pull/1158— newly added by this PR with the same text, attributed to a different PR.The same description text cannot truthfully describe two different PRs. Per
git log 8a174e0, PR #1158 was titled "fix sgl b200/b300 dpsk-v4 script" — a script fix, not a recipe-per-CONC split restore on top of the #1143 fallback. The narrative attached to it in the new entry simply does not match the actual scope of #1158.What this PR should have done
This PR is #1171, titled 'retry sglang b300'. The repo already has a clean precedent for retrigger PRs:
description: 'Retriggering dsv4-fp8-mi355x-sglang',pr-link: .../pull/1148description: 'Retrigger dsv4-fp8-mi355x-sglang',pr-link: .../pull/1160Both use the current PR's own pr-link with a short
Retrigger ...description. Following this precedent, the new entry should be:Step-by-step proof
pr-linkfrom fix sgl b200/b300 dpsk-v4 script #1158 → [NVIDIA] chore: B300 single node DeepSeek v4 SGLang #1132 for the existing entry. This is correct — [NVIDIA] chore: B300 single node DeepSeek v4 SGLang #1132 actually did the recipe-per-CONC restore.git log --onelineshows8a174e0 fix sgl b200/b300 dpsk-v4 script (#1158)— clearly not a recipe-split restore.Addressing the refutation
One verifier noted that the duplicate-description pattern already exists at lines 1837-1846 (#1165) and 1848-1857 (#1170). That counter-example is not load-bearing here: #1165 and #1170 both describe the same DeepSeek-V4-Pro FP4 MI355X ATOM Day-0 marker work — two PRs that genuinely contributed to the same feature. In contrast, #1132 (recipe-per-CONC split restore) and #1158 (script fix) did structurally different things, so reusing #1132's description for #1158 is factual misattribution, not a stylistic duplicate.
Impact and fix
This is a documentation-only file with no runtime impact, so severity is nit. But changelog correctness is load-bearing for downstream attribution in this repo (e.g., #1161 was a dedicated cleanup PR for exactly this kind of misattribution), and the fix is trivial: replace the new entry's description with
"Retrigger dsv4-fp4-b300-sglang"and its pr-link with.../pull/1171.