Skip to content

[AMD][DSV4] Add a DP-attention arm and refresh the MI355X SGLang AgentX key - #2800

Open
karverma-amd wants to merge 10 commits into
mainfrom
karverma_dsv4_0831update
Open

[AMD][DSV4] Add a DP-attention arm and refresh the MI355X SGLang AgentX key#2800
karverma-amd wants to merge 10 commits into
mainfrom
karverma_dsv4_0831update

Conversation

@karverma-amd

@karverma-amd karverma-amd commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a DP-attention arm to dsv4-fp4-mi355x-sglang-agentic-mtp, refreshes the image, enables the DSv4 FP4 indexer, and retunes memory. Settings that differ between the DP and tensor-parallel paths are split per-arm.

Image

  • lmsysorg/sglang-rocm:v0.5.18-rocm720-mi35x-20260829lmsysorg/sglang-rocm:v0.5.18-rocm720-mi35x-20260902

Search space

  • Add { tp: 8, ep: 1, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: hicache }, conc-list: [64, 96, 128, 160] }
  • TP4 arm: drop concurrency 2 and 10, leaving [1, 4, 8]
  • TP8 hicache arm: drop concurrency 16, leaving [32, 48]. Concurrency 16 remains on the TP8 no-offload arm.

10 cells: 6 tensor-parallel, 4 DP.

Serving, all arms

  • --enable-deepseek-v4-fp4-indexer
  • HSA_NO_SCRATCH_RECLAIM=0
  • mem-fraction-static 0.89 → 0.86 on TP4, TP8 and TP8 DP alike

Serving, split per-arm

non-DP dp-attn true
shared experts --enforce-shared-experts-fusion --disable-shared-experts-fusion
swa-full-tokens-ratio 0.10 0.15
GPU_MAX_HW_QUEUES 2 5

Shared-experts fusion is disabled under DP to match the DP baselines, and GPU_MAX_HW_QUEUES=5 is the documented companion to two-batch overlap. All three stay overridable (SWA_FULL_TOKENS_RATIO / SWA_FULL_TOKENS_RATIO_DP, GPU_MAX_HW_QUEUES / GPU_MAX_HW_QUEUES_DP). Previously only the DP branch set GPU_MAX_HW_QUEUES, so the tensor-parallel arms took the container default; both are now explicit.

DP-attention flags, alongside the existing --enable-prefill-delayer:

  • --enable-two-batch-overlap
  • --enable-dp-attention-local-control-broadcast
  • --tokenizer-worker-num equal to TP
  • --stream-interval 20
  • --prefill-decode-interval 10

Chunked prefill under DP now scales the per-TP base (16384 at TP8, 8192 at TP4) by the DP degree, instead of a fixed 8192 * TP.

The DP arm runs behind sglang-router with consistent hashing on the AIPerf correlation id, so multi-turn sessions stay on the DP rank holding their radix/hicache prefix.

Test plan

  • bash -n benchmarks/single_node/agentic/dsv4_fp4_mi355x_sglang_mtp.sh
  • configs/amd-master.yaml and perf-changelog.yaml both parse
  • Config generation yields 10 cells on the new image, 4 of them dp-attn: true
  • All three arm shapes resolve as intended: tp4 and tp8 at mem-frac 0.86 / swa 0.10 / queues 2 / fusion enforced; tp8 DP at mem-frac 0.86 / swa 0.15 / queues 5 / fusion disabled
  • perf-changelog.yaml diff is additions-only, so check-changelog passes
  • process_changelog.py --base-ref main --head-ref HEAD triggers all 10 cells
  • Sweep run on MI355X — the DP arm and the FP4 indexer have not been run on this image

Note on memory headroom

An earlier changelog entry records "Use mem-fraction-static 0.88 for every DP-attention profile while retaining 0.90 for TP profiles. Full c96 and c128 sweeps at 0.90 exhausted HBM in the context-dependent FP4-indexer workspace." This PR lowers to 0.86 with the FP4 indexer enabled and extends the DP arm to concurrency 160, so the c160 point is the one most likely to hit that limit first.


Note

Medium Risk
Benchmark and serving-tuning changes affect HBM headroom (0.86 + FP4 indexer, c160 DP) and introduce an untested-on-hardware DP-attention + router path; no production runtime code paths.

Overview
Refreshes dsv4-fp4-mi355x-sglang-agentic-mtp: bumps the SGLang ROCm image to 20260902, trims several TP concurrency points, and adds a TP8 + DP-attention + HiCache sweep arm at conc [64, 96, 128, 160].

The AgentX launch script now turns on --enable-deepseek-v4-fp4-indexer and HSA_NO_SCRATCH_RECLAIM=0 on all arms, lowers default mem-fraction-static from 0.89 → 0.86, and splits TP vs DP settings: shared-experts fusion enforced on TP but disabled under DP; swa-full-tokens-ratio 0.10 vs 0.15; GPU_MAX_HW_QUEUES 2 vs 5. The DP path adds two-batch overlap, local-control broadcast, tokenizer workers, stream/prefill-decode intervals, scales chunked prefill off the per-TP base (not a fixed 8192×TP), and keeps sglang-router consistent hashing on the AIPerf correlation id.

perf-changelog.yaml documents the matrix and serving changes for sweep invalidation.

Reviewed by Cursor Bugbot for commit 1610f4a. Bugbot is set up for automated code reviews on this repo. Configure here.

…tX key

Image: lmsysorg/sglang-rocm:v0.5.18-rocm720-mi35x-20260829 -> ...-20260831.

Search space:
- add tp 8, ep 1, dp-attn true on the hicache host KV tier at concurrency
  [64, 96, 128]
- drop concurrency 2 and 10 from the TP4 arm, leaving [1, 4, 8]
- drop concurrency 16 from the TP8 hicache arm, leaving [32, 48]; conc 16
  stays on the TP8 no-offload arm

Serving, split per-arm so the tensor-parallel arms keep their current settings:
- shared-experts fusion stays enforced without DP and is disabled under DP
  attention, matching the DP baselines
- swa-full-tokens-ratio stays 0.10 without DP and is 0.15 under DP attention
- DP attention adds --enable-two-batch-overlap,
  --enable-dp-attention-local-control-broadcast, --tokenizer-worker-num equal
  to TP, --stream-interval 20 and --prefill-decode-interval 10 alongside the
  existing --enable-prefill-delayer
- chunked prefill under DP scales the per-TP base (16384 at TP8, 8192 at TP4)
  by the DP degree rather than a fixed 8192
- mem-fraction-static 0.89 -> 0.90

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

Co-authored-by: Cursor <cursoragent@cursor.com>

@claude claude Bot left a comment

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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I checked two other candidate concerns and ruled both out. The widened CHUNKED_PREFILL_SIZE (131072 at TP8) matches the sibling dsv4_fp4_b300_sglang_mtp.sh recipe's own comment that --chunked-prefill-size is a global budget divided per DP rank server-side, so the per-rank size that has to stay under the uint16 cap is unchanged. And pairing --tokenizer-worker-num "$TP" with --dp "$TP" matches the identical pattern already used in both the b200 and b300 single-node DP-attention recipes, not a deviation from convention.

Extended reasoning...

Beyond the already-reported inline finding, I independently verified the two ruled-out candidates against the sibling B200/B300 single-node DP-attention recipes. The chunked-prefill-size math (CHUNKED_PREFILL_SIZE * TP) mirrors dsv4_fp4_b300_sglang_mtp.sh, which carries an explicit comment that server_args.py treats --chunked-prefill-size as a global budget divided across DP ranks, so the effective per-rank chunk size is unchanged and the uint16 cap concern doesn't apply. Similarly, pairing --tokenizer-worker-num "$TP" with --dp "$TP" is exactly the pattern used in both dsv4_fp4_b200_sglang_mtp.sh and dsv4_fp4_b300_sglang_mtp.sh, so this recipe is consistent with, not a deviation from, existing single-node DP-attention conventions.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 benchmarks/single_node/agentic/dsv4_fp4_mi355x_sglang_mtp.sh — The comment above PARALLEL_ARGS still claims the DP-attention path is 'currently DORMANT (no dp-attn arms in amd-master.yaml for this key)', but this same PR adds a dp-attn:true arm in configs/amd-master.yaml, so the comment is now false and will mislead future maintainers debugging or extending the DP branch. Fix: update or remove the dormant-path note now that the arm is live, e.g. describe the router/consistent-hashing rationale without asserting no arm uses it.

    Extended reasoning...

    Lines 123-127 (unchanged by this diff) state the DP-attention branch has no caller today. configs/amd-master.yaml now adds {tp: 8, dp-attn: true, ...}, which sets DP_ATTENTION=true and exercises exactly this branch in production runs. A future engineer reading the stale comment while debugging a DP-arm failure would incorrectly assume the code path is unused and look elsewhere, wasting debugging time; the base branch's comment was accurate when written but this diff invalidates it without updating it.

    Verification: nit — stale comment, nothing functionally breaks. Lines 123-127 of benchmarks/single_node/agentic/dsv4_fp4_mi355x_sglang_mtp.sh (unchanged by the diff) read: "NOTE: the DP-attention path below is currently DORMANT (no dp-attn arms in amd-master.yaml for this key)." The same PR's configs/amd-master.yaml adds `+ { tp: 8, ep: 1, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name:…

main appended five entries (#2770-#2774, #2790) at the end of the changelog
while this branch appended the DSv4 MI355X entry, so the two collided on the
same trailing lines. All are kept, upstream's first.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0ea4013. Configure here.

Comment thread benchmarks/single_node/agentic/dsv4_fp4_mi355x_sglang_mtp.sh
The DP branch previously exported 2, and the tensor-parallel arms did not set
it at all, taking the container default. Both are now explicit: 2 on TP4/TP8
and 5 under DP attention, the documented companion to two-batch overlap.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

The merge resolution reordered main's trailing entries, which check-changelog
rejects ("Deletions are not allowed in perf-changelog.yaml"). Restore main's
file verbatim and append this entry at the end instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

1 similar comment
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

karverma-amd and others added 4 commits September 2, 2026 22:46
…the DP arm

- mem-fraction-static 0.86 on both TP8 arms, with and without DP attention;
  TP4 stays at 0.89
- serve with --enable-deepseek-v4-fp4-indexer
- export HSA_NO_SCRATCH_RECLAIM=0
- DP arm concurrency [64, 96, 128] -> [64, 96, 128, 160]

Co-authored-by: Cursor <cursoragent@cursor.com>
…ons-only

Co-authored-by: Cursor <cursoragent@cursor.com>
Applies the same 0.86 to every arm rather than keeping TP4 at 0.89.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@1am9trash 1am9trash added agentx AgentX benchmarks, recipes, and infrastructure AMD and removed agentx AgentX benchmarks, recipes, and infrastructure labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@1am9trash

1am9trash commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

The run was somehow canceled. Update branch to retrigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants