AMD: MI355x: Lower the GLM-5.2 MI355X HiCache host-pool ratio from 1.5 to 1.0 - #3004
AMD: MI355x: Lower the GLM-5.2 MI355X HiCache host-pool ratio from 1.5 to 1.0#3004jiejingzhangamd wants to merge 1 commit into
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the 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. See GitHub's docs on re-running failed jobs 感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 如需进行 PR 验证,请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
Ratio is relative to the device KV pool, so 1.5 resolved to 169.73 GB/rank on this cluster, or 679 GB pinned across the 4 TP ranks. 1.0 brings that to ~113 GB/rank (~453 GB). Allocation logic is unchanged; eval and sweep still share the same ratio-based sizing.
e1b6cbc to
a6b9d3a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a6b9d3a. Configure here.
| HICACHE_RATIO="${HICACHE_RATIO:-1.5}" | ||
| # ratio=1.0 (~113 GB/rank, ~453 GB pinned at TP4). Raise via the | ||
| # HICACHE_RATIO env-var override on nodes with more DRAM headroom. | ||
| HICACHE_RATIO="${HICACHE_RATIO:-1.0}" |
There was a problem hiding this comment.
Eval HiCache pool not sized absolutely
High Severity
The shared HICACHE_RATIO default dropped from 1.5 to 1.0, so EVAL_ONLY still sizes the host pool by ratio instead of an absolute --hicache-size. GSM8K can still pin hundreds of GB per rank, and the throughput serve shape changes with it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a6b9d3a. Configure here.
| # GSM8K never fills the agentic host pool; ratio 1.5 pins ~679 GB on TP4. | ||
| if [ "${EVAL_ONLY:-false}" = "true" ]; then | ||
| HICACHE_EVAL_SIZE_GB="${HICACHE_EVAL_SIZE_GB:-16}" | ||
| HICACHE_POOL_ARGS=(--hicache-size "$HICACHE_EVAL_SIZE_GB") | ||
| HICACHE_POOL_DESC="size=${HICACHE_EVAL_SIZE_GB} GB/rank (eval-only)" | ||
| else | ||
| HICACHE_POOL_ARGS=(--hicache-ratio "$HICACHE_RATIO" --hicache-size 0) | ||
| HICACHE_POOL_DESC="ratio=$HICACHE_RATIO" | ||
| fi |
There was a problem hiding this comment.
🔴 This recipe change (new EVAL_ONLY HiCache sizing behavior) has no perf-changelog.yaml entry, violating AGENTS.md's append-only changelog rule for changes affecting benchmark performance. Fix: add a perf-changelog.yaml entry for config-key glm5.2-fp4-mi355x-sglang-agentic-mtp describing the eval-only HiCache pool size change, appended at the tail with existing bytes preserved. Precedent: prior EVAL_ONLY-specific OOM/sizing fixes for other recipes (e.g. the EVAL_CONCURRENT_REQUESTS KV-budget cap, DSv4/MTP eval simulated-acceptance splits) all got dedicated changelog entries; this PR and its earlier incarnation (#2983) both omit one.
Extended reasoning...
AGENTS.md line 31: "Every change that can affect benchmark performance and every recipe addition or modification requires a new perf-changelog.yaml entry." This diff modifies the eval-only server launch behavior for glm5.2-fp4-mi355x-sglang-agentic-mtp (host HiCache pool 169.73 GB/rank -> 16 GB/rank for EVAL_ONLY), which is exactly the class of change perf-changelog.yaml exists to track (compare entries like the EVAL_CONCURRENT_REQUESTS=8 KV-cap fix at line 4822 or the DSv4 eval sizing note). No entry was added in this diff (only the .sh file changed), and the PR description explicitly argues no entry is needed because "sweep serve flags are unchanged" -- but the rule is not scoped to sweep-only changes, it covers any change that can affect benchmark performance, and eval-run behavior/repeatability is in scope per the precedent entries. This is the same gap present in the original a5f23f4 commit that this PR relands unchanged.
Verification: Severity: normal — acknowledged in diff, author's "no entry needed" claim does not hold. AGENTS.md line 31 ("Non-negotiable benchmark invariants"): "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." This diff modifies the recipe script… | nit; acknowledged in…


What
--hicache-ratio 1.5→1.0for the TP arm. The DP-attention arm stays at 0.5. No change to the allocation logic: eval and sweep still share the same ratio-based sizing.Why
Ratio is relative to the device KV pool. On
cluster:mi355x-amdsthe TP4 device pool measured 127.90 GB/rank, so ratio 1.5 resolved to 169.73 GB/rank — 679 GB pinned across the 4 TP ranks. Ratio 1.0 brings that to ~113 GB/rank, ~453 GB total.The GSM8K eval for
glm5.2_tp4_conc12_kvdram-hicache_spec-mtpwas SIGKILLed mid-eval onmi355x-amds_07(132625 Killed) oncelm_evalstarted touching those pinned pages: allocation and warmup succeeded, then the OOM killer fired on the first batch.The host tier is also not paying for its size today, so the smaller pool is worth measuring on its own.
Test plan
Eval-only dispatch of the AgentX config (no throughput sweep) to see whether the eval survives at ratio 1.0:
test-config --config-files configs/amd-master.yaml --config-keys glm5.2-fp4-mi355x-sglang-agentic-mtp --evals-only