[AMD] [AGENTX] GLM-5.2 FP4 MI355X ATOM Agentic MTP - #2576
Conversation
Add the MI355X GLM-5.2 FP4 ATOM AgentX recipe with MTP speculative decoding and LMCache DRAM offload, plus the amd-master.yaml config entry glm5.2-fp4-mi355x-atom-agentic-mtp. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add perf-changelog entry for glm5.2-fp4-mi355x-atom-agentic-mtp (PR #2576). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Correct the script header to describe GLM-5.2 FP4 ATOM MTP instead of the leftover MiniMax-M3 vLLM/EAGLE3 template text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename VLLM_CMD to ATOM_CMD and correct the server label to "ATOM server" to reflect the atom.entrypoints.openai_server backend. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
seungrokj
added
AMD
full-sweep-fail-fast
agentx
functionstackx
left a comment
functionstackx
left a comment
There was a problem hiding this comment.
can u edit
to have the atom way of setting fair mtp| SIMULATE_ACC_LEN=2.99 | ||
| NUM_SPEC_TOKENS=3 | ||
| # spec-decode-acceptance-rate = (SIMULATE_ACC_LEN - 1) / NUM_SPEC_TOKENS | ||
| SPEC_ACCEPTANCE_RATE=$(awk "BEGIN{print ($SIMULATE_ACC_LEN-1)/$NUM_SPEC_TOKENS}") |
There was a problem hiding this comment.
can u edit
to have the atom way of setting fair mtpThere was a problem hiding this comment.
@functionstackx
when
--spec-decode-acceptance-rate
is specified then it will fall back to
https://github.com/ROCm/ATOM/blob/57143379cca658d5ae7a08ebb9be8787766d0104/atom/model_ops/rejection_sampler.py#L18-L25
and this will reject based on the synthetic decay factor
https://github.com/ROCm/ATOM/blob/57143379cca658d5ae7a08ebb9be8787766d0104/atom/model_ops/rejection_sampler.py#L395
and they will behave the same as sgl's match-expected behavior
but will cross check with the one who implemented this to make sure
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
configs/amd-master.yaml:1653-1655— Comment forglm5.2-fp4-mi355x-atom-agentic-mtpinconfigs/amd-master.yaml(lines 1653-1655) is truncated mid-sentence: it ends at "...to support" before jumping to the source-PR line. The sibling entryglm5.2-fp4-mi355x-sglang-agentic-mtphas the intended complete phrasing ("...to support long agentic context windows.") — please complete the sentence similarly.Extended reasoning...
The new comment block added just above
glm5.2-fp4-mi355x-atom-agentic-mtpreads:# GLM-5.2 FP4 agentic-coding benchmark on MI355X via ATOM with MTP speculative # decoding. TP=4, TP=8 with KV offloading to DRAM (lmcache backend) to support # Recipe is from PR https://github.com/ROCm/ATOM/pull/1877The second line ends on the dangling preposition "to support" with no object, and the very next comment line abruptly pivots to an unrelated topic (the source-PR attribution). This reads as an editing slip where a clause was cut off before the comment was finalized.
This is clearly a copy/paste-and-edit artifact from the sibling recipe added a few entries earlier in the same file,
glm5.2-fp4-mi355x-sglang-agentic-mtp(lines 1635-1637), whose equivalent comment is complete:# GLM-5.2 FP4 agentic-coding benchmark on MI355X via SGLang with MTP speculative # decoding. TP=4 EP=4 with KV offloading to DRAM (hicache backend) to support # long agentic context windows. Concurrency sweep [1, 2, 4, 8, 10].Here the rationale clause is finished ("...to support long agentic context windows.") before the sweep description. The new ATOM entry was evidently derived from this sibling block but the completing phrase was dropped when the KV-offload backend name was swapped from
hicachetolmcacheand the PR-attribution line was appended.There is no functional or parsing impact: YAML comments are inert, so this does not affect config validation, benchmark scheduling, or CI. The only cost is to a future reader trying to understand why DRAM KV offloading was chosen for this recipe — the rationale (supporting long agentic context windows, i.e. keeping large KV footprints off GPU HBM during long-horizon agentic-coding traces) is left unstated, and the truncation makes the comment look unintentionally broken rather than deliberately terse.
Proof: Reading lines 1653-1655 of
configs/amd-master.yamlas diffed in this PR shows the second comment line terminates immediately after "to support" with a trailing space and no punctuation, and line 1655 starts a grammatically unrelated sentence ("Recipe is from PR ..."). Diffing against the sibling block at lines 1635-1637 (unchanged in this PR, added previously) shows the intended completed form of the same clause structure, confirming what content is missing.Suggested fix: complete the sentence, e.g.:
# GLM-5.2 FP4 agentic-coding benchmark on MI355X via ATOM with MTP speculative # decoding. TP=4, TP=8 with KV offloading to DRAM (lmcache backend) to support # long agentic context windows. # Recipe is from PR https://github.com/ROCm/ATOM/pull/1877This is a comment-only defect with no behavioral effect, so it is a nit.
| @@ -0,0 +1,184 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
🔴 AGENTS.md declares a non-negotiable invariant: every recipe addition requires a new perf-changelog.yaml entry, but this PR adds the brand-new glm5.2-fp4-mi355x-atom-agentic-mtp recipe (script + configs/amd-master.yaml entry) without one. The directly analogous sibling PR (glm5.2-fp4-mi355x-sglang-agentic-mtp) added the required 7-line entry alongside its script/config changes — this PR needs the equivalent append at the tail of perf-changelog.yaml before merge.
Extended reasoning...
The bug: AGENTS.md lines 19-21 list "Non-negotiable benchmark invariants," the first of which states: "Every change that can affect benchmark performance and every recipe addition or modification requires a new perf-changelog.yaml entry. The file is append-only and byte-sensitive. Preserve all existing bytes and separator whitespace, and append only at the tail."
This PR adds a brand-new recipe key, glm5.2-fp4-mi355x-atom-agentic-mtp, to configs/amd-master.yaml, plus the corresponding new benchmark script benchmarks/single_node/agentic/glm5.2_fp4_mi355x_atom_mtp.sh. Both changes squarely fall under "recipe addition" per the invariant. Yet the PR diff touches only these two files — perf-changelog.yaml is not present in the diff at all.
Why this isn't caught elsewhere: There's no CI lint step visible in this repo that mechanically enforces the invariant (it's documented policy, not a script-enforced check), so a reviewer/author has to remember to append the entry manually. The immediately preceding entry in configs/amd-master.yaml, glm5.2-fp4-mi355x-sglang-agentic-mtp (the direct SGLang sibling of this ATOM recipe, same model/precision/SKU/scenario), did add its perf-changelog.yaml entry in its own commit — confirming this is the expected, followed convention for this exact recipe family, not a check nobody actually does in practice.
Impact: Without the changelog entry, there is no recorded baseline/description for this new recipe's expected performance characteristics, which is the entire purpose of the changelog per the AGENTS.md invariant — it's how the team tracks what performance-affecting changes were intentional versus regressions.
Proof / how to verify:
grep -n "atom-agentic-mtp" AGENTS.md configs/amd-master.yaml perf-changelog.yaml— the recipe key appears inconfigs/amd-master.yaml(this PR's diff) but not inperf-changelog.yaml.- Compare against the sibling:
grep -n "sglang-agentic-mtp" perf-changelog.yamlshows the SGLang sibling's entry exists. - The PR's own changed-files list (
benchmarks/single_node/agentic/glm5.2_fp4_mi355x_atom_mtp.sh,configs/amd-master.yaml) confirms only 2 files changed —perf-changelog.yamlis absent.
Fix: Append a new entry to the tail of perf-changelog.yaml (config-keys / description / pr-link, matching the format of the glm5.2-fp4-mi355x-sglang-agentic-mtp entry) for glm5.2-fp4-mi355x-atom-agentic-mtp, being careful to preserve existing bytes/whitespace and append-only per the invariant's own byte-sensitivity warning.
| LMCACHE_LOG="$RESULT_DIR/lmcache_server.log" | ||
| mkdir -p "$RESULT_DIR" | ||
|
|
||
| SERVER_PID="" | ||
| LMCACHE_PIDS=() | ||
| cleanup_agentic_services() { | ||
| local exit_code=$? | ||
| trap - EXIT INT TERM | ||
| set +e | ||
| stop_background_process_tree "$SERVER_PID" "vLLM server" 60 | ||
| local i | ||
| for i in "${!LMCACHE_PIDS[@]}"; do | ||
| stop_background_process_tree "${LMCACHE_PIDS[$i]}" "LMCache server $i" | ||
| done |
There was a problem hiding this comment.
🟡 Dead LMCache server-process scaffolding was copy-pasted from the vLLM script this recipe was adapted from: LMCACHE_LOG (line 52), LMCACHE_PIDS (line 56), and the cleanup loop over LMCACHE_PIDS (lines 63-65) are never written/populated here, since this ATOM recipe offloads KV in-process via the lmcache_offload connector rather than spawning a standalone LMCache server. Recommend deleting these three pieces to avoid implying an external LMCache server process is expected.
Extended reasoning...
This script was adapted from benchmarks/single_node/agentic/minimaxm3_fp4_mi355x_mtp.sh, which actually spawns a standalone lmcache server process in the background, appends its PID to LMCACHE_PIDS (LMCACHE_PIDS+=($!)), and redirects that server's stdout/stderr to LMCACHE_LOG (> "$LMCACHE_LOG"). In that sibling script the log path and PID array are load-bearing: the cleanup trap needs the PIDs to tear down the spawned server, and the log is useful for debugging server startup issues.
This GLM-5.2 ATOM recipe took a different architectural path for KV offload — it configures the in-process lmcache_offload kv_connector via --kv-transfer-config (see OFFLOAD_ARGS in the lmcache case of the KV_OFFLOAD_BACKEND switch), which runs inside the same vLLM/ATOM server process rather than as a separate process. No lmcache server command is ever invoked anywhere in the script. Despite that, the vLLM-script scaffolding for tracking an external server was carried over verbatim:
- Line 52:
LMCACHE_LOG="$RESULT_DIR/lmcache_server.log"— assigned but never referenced again; nothing ever writes to this path. - Line 56:
LMCACHE_PIDS=()— initialized empty and never appended to, since nolmcache serverprocess is spawned. - Lines 63-65: the cleanup loop
for i in "${!LMCACHE_PIDS[@]}"; do stop_background_process_tree "${LMCACHE_PIDS[$i]}" "LMCache server $i"; done— iterates a permanently empty array, so it is a runtime no-op on every invocation, forever.
Step-by-step proof: (1) LMCACHE_PIDS=() sets the array empty at line 56. (2) Grep the rest of the file for LMCACHE_PIDS+= or any other mutation — there is none; the lmcache case of the KV_OFFLOAD_BACKEND switch only sets OFFLOAD_ARGS (a --kv-transfer-config flag passed to the single VLLM_CMD/SERVER_PID process) and never runs lmcache server & or captures a $! into LMCACHE_PIDS. (3) Therefore at cleanup time "${!LMCACHE_PIDS[@]}" expands to nothing, and the for loop body executes zero iterations on every run, regardless of KV_OFFLOAD_BACKEND. (4) Similarly, LMCACHE_LOG is assigned once at line 52 and grep shows no other reference to it in the file — no > "$LMCACHE_LOG" redirection exists anywhere, unlike the vLLM script it was copied from.
This has no functional/runtime impact — an empty-array loop is harmless — but it is misleading dead code: a future maintainer reading this script would reasonably assume an external LMCache server process is started and tracked here, when in fact KV offload is entirely in-process. Recommend removing LMCACHE_LOG, LMCACHE_PIDS, and the associated cleanup loop (or, if a future revision needs the standalone LMCache server path, keep them but actually spawn/track that process as the vLLM sibling script does).
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31613244164 |
… TOTAL_CPU_DRAM_GB Set LMCACHE_MAX_LOCAL_CPU_SIZE to the harness-provided TOTAL_CPU_DRAM_GB instead of a hardcoded 200 GB, so the DRAM offload budget tracks the config's dram-utilization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31615355795 |
…ation to 0.064 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ding for tp8 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31654183817 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31654311672 |
2 participants
Summary
benchmarks/single_node/agentic/glm5.2_fp4_mi355x_atom_mtp.sh) with MTP speculative decoding and LMCache DRAM offload.glm5.2-fp4-mi355x-atom-agentic-mtpconfig inconfigs/amd-master.yaml(TP4/TP8, lmcache DRAM offload, MTP, agentic-coding scenario).ptpc_fp8online quant,lmcache_offloadconnector,--method mtp, acceptance rate derived from the golden AL distribution).Test plan
bash -n benchmarks/single_node/agentic/glm5.2_fp4_mi355x_atom_mtp.shpassesglm5.2-fp4-mi355x-atom-agentic-mtp🤖 Generated with Claude Code