Skip to content

[DEBUG] remove qwen3.5-fp8-mi355x-atom perf-changelog entry#1296

Open
seungrokj wants to merge 1 commit intomainfrom
srok/atom_qwen3.5_fp8_fix
Open

[DEBUG] remove qwen3.5-fp8-mi355x-atom perf-changelog entry#1296
seungrokj wants to merge 1 commit intomainfrom
srok/atom_qwen3.5_fp8_fix

Conversation

@seungrokj
Copy link
Copy Markdown
Collaborator

@seungrokj seungrokj commented May 7, 2026

Hi @functionstackx @cquil11

this PR is to resolve this issue:
https://github.com/SemiAnalysisAI/InferenceX/actions/runs/24868987151/job/72811203316

and https://github.com/SemiAnalysisAI/InferenceX/blob/main/utils/process_changelog.py
is incompatible with this mis-aligned config-keys: key

image

So once you merge this, then I'll add them back again. (without mis-aligment)

==

Summary

Test plan

  • Verify perf-changelog.yaml no longer has duplicate entries for qwen3.5-fp8-mi355x-atom

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@seungrokj seungrokj requested a review from a team May 7, 2026 09:37
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 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.

Comment thread perf-changelog.yaml
Comment on lines 1436 to 1441
- "Image includes upstream SGLang PRs: https://github.com/sgl-project/sglang/pull/21188, https://github.com/sgl-project/sglang/pull/21421, https://github.com/sgl-project/sglang/pull/20736"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1036

- config-keys:
- qwen3.5-fp8-mi355x-atom
- qwen3.5-fp8-mi355x-atom-mtp
description:
- "Add Qwen3.5-397B-A17B FP8 MI355X ATOM benchmark configs with and without MTP"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1040


- config-keys:
- qwen3.5-fp4-mi355x-sglang
description:
Copy link
Copy Markdown
Contributor

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 actual dsv4-fp4-mi355x-atom duplicate 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-mtp that 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 contains qwen3.5-fp8-mi355x-atom / -mtp (lines 1439-1444):

- config-keys:
    - qwen3.5-fp8-mi355x-atom
    - qwen3.5-fp8-mi355x-atom-mtp
  description:
    - 'Add Qwen3.5-397B-A17B FP8 MI355X ATOM benchmark configs with and without MTP'
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1040

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 unrelated dsv4-fp4-mi355x-atom entries ("...matches qwen3.5-fp8-mi355x-atom base..."), not config-key entries.

Step-by-step proof

  1. 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.
  2. git show HEAD~1:perf-changelog.yaml | sed -n '1439,1444p' → shows the block has pr-link: .../pull/1040. So the "existing entry referencing PR [AMD/ROCM] atom qwen fp8/fp8_mtp3 on mi355x #1040" is this block.
  3. git log --all --oneline -S 'qwen3.5-fp8-mi355x-atom' -- perf-changelog.yaml → only a70a094 (file creation) ever added the token as a config-key, and c11b281 (this PR) removed it. No commit ever added a duplicate.
  4. Post-PR: grep -nE '^\s+- qwen3.5-fp8-mi355x-atom($|-mtp$)' perf-changelog.yaml → 0 hits.
  5. The configs still exist in .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-atom appears 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 those dsv4-fp4-mi355x-atom blocks 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-atom and qwen3.5-fp8-mi355x-atom-mtp were 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 actual dsv4-fp4-mi355x-atom duplicate at file lines 1832/1843, that should be done in a separate PR that explicitly targets that config-key.

@seungrokj seungrokj changed the title Remove duplicate qwen3.5-fp8-mi355x-atom perf-changelog entry [DEBUG] remove qwen3.5-fp8-mi355x-atom perf-changelog entry May 7, 2026
@seungrokj seungrokj enabled auto-merge (squash) May 7, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant