AMD AgentX MI355X: GPU sanity dedup, HiCache tuning, node-exclude scoping, and DSv4 disagg image bump - #2643
AMD AgentX MI355X: GPU sanity dedup, HiCache tuning, node-exclude scoping, and DSv4 disagg image bump#2643ichbinblau wants to merge 9 commits into
Conversation
…_gpu_clean Removes helpers/gpu_sanity.sh, whose VRAM-drain check duplicated the wait_for_amd_gpu_clean() gate added to benchmark_lib.sh in #2490. Wires that shared function into job.slurm's pre-flight gate instead, guarding against benchmark_lib.sh's unrelated agentic KV_OFFLOADING validation by unsetting IS_AGENTIC/SCENARIO_TYPE before sourcing it standalone. Co-authored-by: Cursor <cursoragent@cursor.com>
…he image - models.yaml (DeepSeek-V4-Pro-AgentX): move swa-full-tokens-ratio from a global base_flag to per-role prefill/decode fields, add --enable-two-batch-overlap to dp_flags, raise prefill mem_fraction_static to 0.85 and no_dp max_running_requests to 128, enable decode CUDA graphs. - dsv4_fp4_mi355x_sglang-disagg.sh: lower default HICACHE_RATIO to 3 and trim the FORCE_HICACHE_RATIO rationale comment. - amd-master.yaml: bump the dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp image, extend the MTP sweep conc-list to include 64, and comment out the superseded TP8/EP1 single-worker search-space arm. Co-authored-by: Cursor <cursoragent@cursor.com>
…xed global list Known-bad nodes only affect Kimi-K2.5-MXFP4 on vllm-disagg; other framework/model combos no longer get any nodes excluded by default. Mapping lives in node_excludes.yaml; SLURM_EXCLUDE_NODES still overrides it. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Theresa Shan <theresa.shan@amd.com> bump image to aigmkt/mori-dev:sglang-0.5.17-rocm720-mi35x-mori-0814 Signed-off-by: Theresa Shan <theresa.shan@amd.com> add one more sweep dot Signed-off-by: Theresa Shan <theresa.shan@amd.com> Update amd-master.yaml
|
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 关于重新运行失败任务的文档 |
1 similar comment
|
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 关于重新运行失败任务的文档 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approving — the three findings are all self-contained nits (silent-failure diagnostics gap, a sensitivity regression with an existing escape hatch, and a dead config key) rather than functional blockers, and the PR is benchmark/tuning infra rather than production-serving code.
Extended reasoning...
Overview
This PR touches AMD multi-node benchmark tooling only: it wires the shared wait_for_amd_gpu_clean helper into job.slurm's pre-flight gate (replacing the standalone gpu_sanity.sh), tunes DeepSeek-V4-Pro-AgentX HiCache/CUDA-graph settings in models.yaml, scopes submit.sh's node exclusions to specific (framework, model) combos via a new node_excludes.yaml, and bumps an image tag plus extends a concurrency sweep in configs/amd-master.yaml. No production inference-serving code, auth, or data-handling paths are touched.
Security risks
None. This is CI/benchmark orchestration bash and YAML config running on internal SLURM/Docker infrastructure the author controls; there's no user-facing input, auth, or data exposure surface. The python3 -c node_excludes lookup interpolates values from the same repo's YAML/env vars, not external/untrusted input.
Level of scrutiny
Medium — this is benchmark tooling (not production-critical), but it does touch a safety gate (GPU drain check) and a known-bad-node exclusion mechanism, so correctness matters for CI reliability even though a bad merge here only wastes benchmark runs rather than affecting production traffic. The bug-hunting pass found three issues, all explicitly scoped as nits: a silent-failure mode if python3/PyYAML is missing on the bare submit host (mitigated by an existing identical pattern elsewhere in the repo), a reduction in the GPU-drain gate's absolute-GB sensitivity when switched to the shared percentage-based helper (mitigated by the existing SKIP_GPU_SANITY escape hatch and the fact it only misses a narrow 8-28GB band), and a dead/unwired decode.disable_cuda_graph config key that is a no-op today since its value is false (matching current always-on behavior).
Other factors
None of the three findings represent a functional regression that would break CI or produce incorrect benchmark results today; they're robustness/precision gaps worth fixing but not blocking. The PR is otherwise a straightforward consolidation (removing a duplicate GPU check) and parameter tuning change following established patterns in this file.
…quests to 64 Restores the pre-2fd1ce507 value; 128 was too aggressive for the no_dp prefill role. Co-authored-by: Cursor <cursoragent@cursor.com>
… trusted Checks python3/PyYAML availability upfront and the exit status of the python3 invocation, falling back to an awk parser (mirroring job.slurm's style) when python3/PyYAML are unavailable. Previously a missing python3 or yaml module would silently yield an empty exclude list, reintroducing the known-bad-node scheduling issue with zero diagnostic. Co-authored-by: Cursor <cursoragent@cursor.com>
…ek-V4-Pro-AgentX decode Decode CUDA graphs are enabled by default; the explicit false was a no-op alongside the existing dp/no_dp cuda_graph_bs_range settings. Co-authored-by: Cursor <cursoragent@cursor.com>
ichbinblau
added
AMD
agentx
1 participant
Summary
wait_for_amd_gpu_clean()intojob.slurm's pre-flight gate and removes the now-duplicatehelpers/gpu_sanity.shVRAM-drain check.swa-full-tokens-ratio,--enable-two-batch-overlap, prefillmem_fraction_static, decode CUDA graphs) and lowers the defaultHICACHE_RATIO.submit.shnode exclusions to the (framework, model) combos that actually need them (Kimi-K2.5-MXFP4 on vllm-disagg) vianode_excludes.yaml, instead of excluding known-bad nodes globally.dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtpSGLang image and merges the duplicate spec-none/MTP config entries inamd-master.yamlinto one, extending the MTP concurrency sweep.Test plan
job.slurmpre-flight GPU-drain gate still triggers correctly withhelpers/gpu_sanity.shremovedsubmit.shonly excludes nodes for Kimi-K2.5-MXFP4 + vllm-disagg, andSLURM_EXCLUDE_NODESstill overridesnode_excludes.yamldsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtpe2e sweep to validate the merged config and new imageMade with Cursor