Skip to content
Closed
Changes from 1 commit
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
9 changes: 6 additions & 3 deletions configs/amd-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1703,8 +1703,10 @@ minimaxm3-fp4-mi355x-vllm-agentic-mtp:
# concurrency; GPU-resident KV keeps latency independent of DRAM bandwidth.
# c6 and c8 removed after sweep validation: dominated by TP4/EP4 arm.
# SA selects the Pareto-optimal arm per concurrency point.
# v0.5.19 image bump: includes SGLang #37124 (fused DSA metadata kernels) + #37133.
# Full Pareto re-sweep: TP4/EP4 no-KV, TP8/EP1 no-KV, TP4/EP4 HiCache.
Comment on lines 1703 to +1707

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.

🟡 nit (optional): The stale comment block above this recipe (lines 1697-1705) still describes the old 2-arm structure with conc-lists [1,2,4,8,10,12] and [4,10], directly contradicting both the new 3-arm search-space and the new explanatory lines just added (1706-1707) above it. A maintainer reading top-to-bottom gets conflicting arm counts/conc values. Fix: update or remove the stale block to match the new 3-arm TP4/EP4-noKV, TP8/EP1-noKV, TP4/EP4-HiCache structure and its actual conc-lists.

Extended reasoning...

Lines 1697-1705 (untouched by this diff) narrate 'Two arms' with specific concurrency sweeps [1,2,4,8,10,12] and [4,10], and a rationale ('c6 and c8 removed... dominated by TP4/EP4 arm') that no longer matches the code below. The diff appends two new comment lines (1706-1707) directly beneath it summarizing the real 3-arm change, so the two comment blocks now sit back-to-back describing mutually exclusive designs for the same recipe. A future engineer editing this recipe (e.g. per AGENTS.md's 'follow the nearest existing pattern') could reasonably follow the stale block's numbers instead of the actual search-space, since nothing marks it superseded.

Verification: nit: The comment block at lines 1697-1705 (untouched) still narrates the old 2-arm design: "Two arms: (1) TP4/EP4 with HiCache... concurrency sweep [1, 2, 4, 8, 10, 12]; (2) TP8/EP1 without KV offloading, concurrency sweep [4, 10];" and "c6 and c8 removed after sweep validation: dominated by TP4/EP4 arm." The new search-space (lines 1720-1722) is 3 arms: TP4/EP4 no-KV conc [1,2,4,6,8],…

glm5.2-fp4-mi355x-sglang-agentic-mtp:
image: lmsysorg/sglang-rocm:v0.5.16-rocm720-mi35x-20260728
image: lmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260908
model: amd/GLM-5.2-MXFP4
model-prefix: glm5.2
runner: cluster:mi355x-amds
Expand All @@ -1715,8 +1717,9 @@ glm5.2-fp4-mi355x-sglang-agentic-mtp:
agentic-coding:
- dram-utilization: 0.8
search-space:
- { tp: 4, ep: 4, kv-offloading: dram, kv-offload-backend: { name: hicache }, conc-list: [1, 2, 4, 8, 10, 12], spec-decoding: mtp }
- { tp: 8, ep: 1, kv-offloading: none, conc-list: [1, 2, 4, 10], spec-decoding: mtp }
- { tp: 4, ep: 4, kv-offloading: none, conc-list: [1, 2, 4, 6, 8], spec-decoding: mtp }
- { tp: 8, ep: 1, kv-offloading: none, conc-list: [1, 2, 4], spec-decoding: mtp }
- { tp: 4, ep: 4, kv-offloading: dram, kv-offload-backend: { name: hicache }, conc-list: [1, 2, 4, 8, 10], spec-decoding: mtp }
Comment thread
cursor[bot] marked this conversation as resolved.
# GLM-5.2 FP4 agentic-coding benchmark on MI355X via ATOM with MTP speculative
# decoding. TP4 uses LMCache DRAM offload; TP8 is GPU-resident with no KV offload.
# Recipe is from PR https://github.com/ROCm/ATOM/pull/1877

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 diff bumps the sglang image (v0.5.16->v0.5.19) and restructures the agentic-coding search-space (2 arms -> 3 arms, changed conc-lists) for glm5.2-fp4-mi355x-sglang-agentic-mtp, but perf-changelog.yaml is not touched anywhere in the commit. AGENTS.md's non-negotiable invariant requires every image bump and recipe search-space modification to append a new perf-changelog.yaml entry (append-only, tail). Fix: add a perf-changelog.yaml entry documenting the image bump (old/new versions) and the search-space restructuring before merge.

Extended reasoning...

AGENTS.md line 31 states every change that can affect benchmark performance or recipe modification requires a new perf-changelog.yaml entry, appended at the tail, and the file is treated as byte-sensitive/append-only history of perf-relevant changes. This PR changes the container image (affecting kernel/runtime behavior per the PR's own claimed +18-19% perf impact) and rewrites the search-space arms/conc-lists for a benchmark recipe, yet git diff shows no changes to perf-changelog.yaml at all. The PR description even lists 'Update perf-changelog.yaml with final Pareto values before merge' as an unchecked TODO in the test plan, confirming the entry is missing at HEAD. Merging as-is leaves the changelog silently out of sync with the recipe history, breaking downstream tooling or reviewers that rely on perf-changelog.yaml as the authoritative record of performance-affecting changes.

Verification: normal. The diff (git diff e5f3e41..HEAD) touches only configs/amd-master.yaml. Lines 1707-1708 change image: lmsysorg/sglang-rocm:v0.5.16-rocm720-mi35x-20260728 -> v0.5.19-rocm720-mi35x-20260908 (an image bump), and lines 1720-1722 rewrite the agentic-coding search-space from two arms ([1,2,4,8,10,12] hicache + [1,2,4,10] TP8) to three arms (TP4/EP4 no-KV [1,2,4,6,8], TP8/EP1 no-KV [1,2,4],…

Expand Down