Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion benchmarks/single_node/minimaxm2.5_fp8_mi325x.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ if [ -n "$ROCR_VISIBLE_DEVICES" ]; then
export HIP_VISIBLE_DEVICES="$ROCR_VISIBLE_DEVICES"
fi

# following AMD andy's recipe
# following AMD andy's recipe
# https://www.linkedin.com/posts/andyluo77_day-0-support-of-minimax-25-on-amd-gpu-activity-7428151527309025280-hXR8/
export VLLM_ROCM_USE_AITER=1
export VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1

SERVER_LOG=/workspace/server.log
PORT=${PORT:-8888}
Expand All @@ -52,6 +53,7 @@ $EP \
--max-model-len $MAX_MODEL_LEN \
--block-size=32 \
--no-enable-prefix-caching \
--attention-backend "ROCM_AITER_FA" \
--trust-remote-code > $SERVER_LOG 2>&1 &

SERVER_PID=$!
Expand Down
6 changes: 6 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3068,3 +3068,9 @@
description:
- "Bump image to rocm/sgl-dev:rocm720-mi35x-8c3b5aa-20260521-DSv4"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1548

- config-keys:
- minimaxm2.5-fp8-mi325x-vllm
description:
- "Add VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1 + --attention-backend ROCM_AITER_FA (match AMD-recommended AITER recipe pattern used on mi355x)"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1549
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.

🟡 The new perf-changelog.yaml entry at line 3076 has pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — the literal placeholder XXX was never replaced with 1549. This 404s and breaks any tooling that parses pr-link → merged PR id. Trivial fix: replace XXX with 1549.

Extended reasoning...

What the bug is

The newly added entry in perf-changelog.yaml (lines 3071-3076) for this PR uses the literal placeholder string XXX in its pr-link field rather than the actual PR number 1549:

- config-keys:
    - minimaxm2.5-fp8-mi325x-vllm
  description:
    - "Add VLLM_ROCM_SHUFFLE_KV_CACHE_LAYOUT=1 + --attention-backend ROCM_AITER_FA (match AMD-recommended AITER recipe pattern used on mi355x)"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX

Note on apparent contradiction with the PR diff: The PR diff rendered above shows pull/1549 in the added line, but that is a substitution performed by the PR-rendering tooling — the actual on-disk / committed content is pull/XXX. This was verified two ways: (1) Read perf-changelog.yaml returns pull/XXX on line 3076, and (2) git show c813cb4 -- perf-changelog.yaml shows the committed hunk contains pull/XXX. The PR description itself correctly references #1549, so the author clearly intended 1549 and simply forgot to substitute the placeholder before committing.

Why the convention is clear

Every other entry in perf-changelog.yaml uses a real numeric PR id — pull/1548, pull/1546, pull/1521, pull/1411, etc. There is no precedent for XXX as an intentional sentinel anywhere in the file. This is unambiguously an unsubstituted template, not project convention.

Impact

  1. The changelog navigation link https://github.com/SemiAnalysisAI/InferenceX/pull/XXX 404s — anyone clicking the link in the changelog gets a broken page.
  2. Any tooling that parses pr-link to associate config-keys with merged PR numbers (e.g., for generating release notes, dashboards, or change-attribution reports) will either fail or produce garbage on this entry.

Step-by-step proof

  1. Open perf-changelog.yaml and jump to line 3076. The line reads exactly: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.
  2. Run git show c813cb4 -- perf-changelog.yaml | tail. The added hunk includes + pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — confirming the placeholder is in the committed blob, not a working-tree edit.
  3. Try the URL https://github.com/SemiAnalysisAI/InferenceX/pull/XXX — it 404s (XXX is not a valid PR number).
  4. Compare with the prior entry at line 3070: pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1548 — numeric, valid, matches the file-wide convention.

Fix

One-character (well, three-character) edit on perf-changelog.yaml:3076:

-  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX
+  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1549

Severity is nit — the recipe change itself (the .sh file) is fine; this is purely changelog hygiene. Worth flagging because the broken link is permanent in git history if not fixed before merge.

Loading