[DEBUG] remove qwen3.5-fp8-mi355x-atom perf-changelog entry #1296
+0
−8
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.
🔴 This PR's premise is incorrect — the removed block is not a duplicate, it is the only changelog entry for
qwen3.5-fp8-mi355x-atom/qwen3.5-fp8-mi355x-atom-mtp(and it is the entry referencing PR #1040). After this PR merges, perf-changelog.yaml has zero entries documenting these configs, while the configs themselves still exist in.github/configs/amd-master.yaml. The PR should be reverted; the author may have been thinking of the actualdsv4-fp4-mi355x-atomduplicate at lines 1832/1843, which is a different config-key.Extended reasoning...
What the bug is
The PR description claims it removes a duplicate perf-changelog entry for
qwen3.5-fp8-mi355x-atom/qwen3.5-fp8-mi355x-atom-mtpthat was "accidentally included alongside the existing entry referencing PR #1040." This premise is factually wrong: the deleted block is the only entry for those config-keys, and it is the entry referencing PR #1040. There was no second entry alongside it.How it manifests
Pre-PR (
git show HEAD~1:perf-changelog.yaml), exactly one config-keys list item containsqwen3.5-fp8-mi355x-atom/-mtp(lines 1439-1444):This is precisely the block the PR deletes. Post-PR,
grep '- qwen3.5-fp8-mi355x-atom$'on perf-changelog.yaml returns 0 hits as a config-key. The remaining matches at lines 1835/1846 are inside description strings of unrelateddsv4-fp4-mi355x-atomentries ("...matches qwen3.5-fp8-mi355x-atom base..."), not config-key entries.Step-by-step proof
git show HEAD~1:perf-changelog.yaml | grep -nE '^\s+- qwen3.5-fp8-mi355x-atom($|-mtp$)'→ exactly two lines (the config-keys list items at lines 1440-1441), belonging to a single block.git show HEAD~1:perf-changelog.yaml | sed -n '1439,1444p'→ shows the block haspr-link: .../pull/1040. So the "existing entry referencing PR [AMD/ROCM] atom qwen fp8/fp8_mtp3 on mi355x #1040" is this block.git log --all --oneline -S 'qwen3.5-fp8-mi355x-atom' -- perf-changelog.yaml→ onlya70a094(file creation) ever added the token as a config-key, andc11b281(this PR) removed it. No commit ever added a duplicate.grep -nE '^\s+- qwen3.5-fp8-mi355x-atom($|-mtp$)' perf-changelog.yaml→ 0 hits..github/configs/amd-master.yaml(lines 265 and 288), so they are real, exercised configs whose changelog provenance has now been wiped.Why the premise is plausible-sounding but wrong
There is a real duplicate in the file:
dsv4-fp4-mi355x-atomappears as a config-keys entry at both lines 1832 and 1843 of the post-PR file. The author may have noticed that and confused it with the qwen entry — the description strings on thosedsv4-fp4-mi355x-atomblocks textually mention "qwen3.5-fp8-mi355x-atom base" (referring to the shared docker image), which makes the config-key appear to occur multiple times to a casual grep. But those textual mentions are not config-keys list items.Impact
The changelog now has no entry documenting when/how
qwen3.5-fp8-mi355x-atomandqwen3.5-fp8-mi355x-atom-mtpwere added. There is repo precedent that this matters: PR #1287/#1293 ("Re-append perf-changelog entry for minimaxm2.5-fp8-mi355x-vllm") was filed specifically to restore a similarly missing changelog entry. The PR does the opposite of its stated intent.How to fix
Revert this PR (
git revert c11b281), restoring the lines 1439-1444 block. If the author intended to clean up the actualdsv4-fp4-mi355x-atomduplicate at file lines 1832/1843, that should be done in a separate PR that explicitly targets that config-key.