Skip to content

[AMD][AgentX] Add DeepSeek-V4-Pro FP4 MI355X ATOM MTP - #2698

Merged
cquil11 merged 2 commits into
mainfrom
amd/agentx_dsv4_atom_0821
Aug 20, 2026
Merged

[AMD][AgentX] Add DeepSeek-V4-Pro FP4 MI355X ATOM MTP#2698
cquil11 merged 2 commits into
mainfrom
amd/agentx_dsv4_atom_0821

Conversation

@seungrokj

@seungrokj seungrokj commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Internalizes fork PR #2668 (by @yhl-amd) onto a branch in this repo so full-sweep CI can run on it, plus a follow-up flag update. Changes:

  • New recipe benchmarks/single_node/agentic/dsv4_fp4_mi355x_atom_mtp.sh (+131) — MI355X DeepSeek-V4-Pro FP4 ATOM AgentX MTP.
  • configs/amd-master.yaml (+16) — config entry dsv4-fp4-mi355x-atom-agentic-mtp.
  • perf-changelog.yaml — changelog entry (image bumped to nightly_202608201032).

MTP acceptance flag

The throughput arm now passes ATOM's --spec-decode-acceptance-length 2.49 directly (per ROCm/ATOM#1948) instead of the previously derived --spec-decode-acceptance-rate 0.4966666667. In dsv4_fp4_mi355x_atom_mtp.sh:

  • SPEC_DECODE_AL=2.49 — the committed DeepSeek-V4 thinking-mode golden AL with 3 draft tokens.
  • The flag is only added for throughput runs; EVAL_ONLY=true runs omit it so they measure real MTP acceptance.
SPEC_ARGS+=(--spec-decode-acceptance-length "$SPEC_DECODE_AL")   # throughput only

Applied as the PR's net diff (origin/main...pr-2668) on top of latest main, so no unrelated changes to the shared amd-master.yaml / perf-changelog.yaml are reverted.

Test plan

  • Full-sweep CI green for dsv4-fp4-mi355x-atom-agentic-mtp.

Supersedes/mirrors #2668.

🤖 Generated with Claude Code

Internalizes fork PR #2668 (yhl-amd) so full-sweep CI can run on it.
Adds the MI355X DeepSeek-V4-Pro FP4 ATOM AgentX MTP recipe plus its
amd-master.yaml config entry and perf-changelog entry.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

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 关于重新运行失败任务的文档

1 similar comment
@github-actions

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 关于重新运行失败任务的文档

@seungrokj seungrokj added AMD agentx AgentX benchmarks, recipes, and infrastructure full-sweep-enabled labels Aug 20, 2026
Switch the MTP throughput arm to --spec-decode-acceptance-length 2.49
(ROCm/ATOM#1948) instead of the derived --spec-decode-acceptance-rate,
bump the ATOM image to nightly_202608201032, and point the changelog
pr-link at #2698.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Comment thread perf-changelog.yaml
Comment on lines +6199 to 6211
- config-keys:
- dsv4-fp4-mi355x-atom-agentic-mtp
scenario-type:
- agentic-coding
description:
- "Add DeepSeek-V4-Pro FP4 ATOM MTP AgentX on one 8x MI355X node at concurrency 1, 2, 4, 8, 16, 32, and 48, with no KV offload and max-num-seqs set to twice concurrency."
- "Use rocm/atom-dev:nightly_202608181633 (digest sha256:fdc5650f2b6d13c22f1f1b3873ee6dc61278e6b0b90dc24d0f6c55810895032f), FP8 KV/index caches, prefix caching, 32K state checkpoints, 16K batching/prefill chunks, and FULL cudagraph mode."
- "Use three-token MTP with the committed DeepSeek-V4 thinking-mode golden AL 2.49 (synthetic acceptance rate 0.4966666667) for throughput, while eval-only runs measure real MTP acceptance."
- "Increase the AgentX warmup grace period from the 1800-second default to 3600 seconds at concurrency 32 and 48, matching the DeepSeek-V4-Pro SGLang saturation recipe so in-flight warmup requests can drain and reused long prefixes are fully primed before profiling."
- "Keep concurrency 1 through 16 at the default 1800 seconds; the profiling duration remains 3600 seconds for every concurrency arm."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2668

- config-keys:

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.

🔴 AGENTS.md requires perf-changelog.yaml to be append-only, with new entries appended only at the tail — this entry (dsv4-fp4-mi355x-atom-agentic-mtp, lines 6199-6210) is instead inserted mid-file, immediately before the pre-existing #2672 entry, even though five newer entries (#2672, #2642, #2660, #2639, #2677, #2686) already exist past that point on main. This will fail utils/validate_perf_changelog.py's validate_raw_change(), which requires the new file to start with the old file's exact bytes before the appended suffix. Fix by moving this 12-line block to the true tail of the file (after the #2686 entry at line 6265).

Extended reasoning...

What the bug is. perf-changelog.yaml is explicitly documented in AGENTS.md (line 21, under "Non-negotiable benchmark invariants") as append-only and byte-sensitive: "Preserve all existing bytes and separator whitespace, and append only at the tail." This PR's new dsv4-fp4-mi355x-atom-agentic-mtp entry is inserted at lines 6199-6210, directly after the existing glm5.2-fp4-mi355x-atom-agentic-mtp / #2576 entry — but that is not the tail of the file. The file is 6265 lines long, and the pre-existing kimik3-fp4-b200-dynamo-vllm-agentic-dspark (#2672) entry immediately follows the new insertion, with several more entries after it (#2642, #2660, #2639, #2677, #2686) all the way to the real tail.

Root cause. Per the PR description, this PR "internalizes fork PR #2668 ... Applied as the PR's net diff (origin/main...pr-2668) on top of latest main." When #2668 was originally authored, the entry after #2576 genuinely was the tail of the changelog. But main has since grown past that point (entries for #2672, #2642, #2660, #2639, #2677, #2686 were all appended in the interim). Replaying #2668's diff hunk verbatim (@@ -6196,6 +6196,18 @@) reproduces the same insertion point instead of re-targeting the current EOF, landing the new entry mid-file.

Why nothing else catches this. The YAML itself still parses fine (list order doesn't affect config-keys lookups), so there's no functional regression a benchmark run would surface. But this repo has a dedicated enforcement mechanism: utils/validate_perf_changelog.py's validate_raw_change() (line 211) requires head_raw.startswith(base_raw) — i.e. the new file's bytes must extend the old file's bytes verbatim, with new content only as a trailing suffix starting with - config-keys:. Because this PR's base (pre-#2668-replay) already contained the #2672...#2686 entries after the insertion point, the new file's bytes diverge from the base before EOF, so head_raw.startswith(base_raw) is false and the validator raises "appended entries changed historical perf-changelog.yaml bytes; restore the base file byte-for-byte and append at the end."

Step-by-step proof:

  1. Current main (base) has, in order: ...#2576 entry (ends ~line 6202) -> #2672 entry -> #2642 -> #2660 -> #2639 -> #2677 -> #2686 entry (EOF at line 6247, pre-PR length).
  2. This PR's diff hunk @@ -6196,6 +6196,18 @@ inserts the new 12-line dsv4-fp4-mi355x-atom-agentic-mtp block right after the #2576 entry, i.e. before the #2672 entry.
  3. Resulting head file: ...#2576 -> new dsv4 entry -> #2672 -> #2642 -> ... -> #2686 (now 6265 lines total).
  4. validate_raw_change computes head_raw.startswith(base_raw). Since base_raw's byte sequence continues #2576 -> #2672 -> ... directly (no gap), but head_raw has the new block spliced in between, the two diverge at that splice point — well before either file's end. The check fails and raises the ChangelogValidationError.
  5. Confirmed by direct inspection: wc -l perf-changelog.yaml = 6265; the new entry occupies lines 6199-6210, immediately followed by the #2672 kimik3-fp4-b200-dynamo-vllm-agentic-dspark entry at line 6212, with #2642/#2660/#2639/#2677/#2686 entries following through to line 6265.

The fix is trivial: cut the 12-line dsv4-fp4-mi355x-atom-agentic-mtp block (lines 6199-6210) and paste it at the true current tail, after the #2686 minimaxm3-fp4-b200-trtllm-agentic-mtp entry (end of file, line 6265), preserving all other bytes untouched. No refutations were raised against this finding by other verifiers.

export AIPERF_LIVE_FAILED_REQUEST_THRESHOLD=0.10
export AIPERF_TRACE_IDLE_GAP_CAP_SECONDS=300
export AIPERF_WARMUP_REQUESTS_PER_LANE=10
export AIPERF_BENCHMARK_GRACE_PERIOD=30

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.

🔴 AIPERF_BENCHMARK_GRACE_PERIOD (line 58) isn't a real config var — benchmark_lib.sh only honors AGENTIC_WARMUP_GRACE_PERIOD (default 1800s) for --warmup-grace-period, and this name is referenced nowhere else in the repo. As a result, the perf-changelog's claim of a 3600s warmup grace period at concurrency 32/48 (mirroring dsv4_fp4_mi355x_sglang_mtp.sh's conditional) is never implemented — every arm silently runs at the 1800s default.

Extended reasoning...

The bug: dsv4_fp4_mi355x_atom_mtp.sh:58 sets export AIPERF_BENCHMARK_GRACE_PERIOD=30, but this environment variable is never consumed anywhere in the repository. A repo-wide grep confirms it appears only on this one line. The warmup grace period actually used by the replay harness is controlled by a different variable, AGENTIC_WARMUP_GRACE_PERIOD, which benchmark_lib.sh:2044 reads via --warmup-grace-period ${AGENTIC_WARMUP_GRACE_PERIOD:-1800}. Since the new script never sets AGENTIC_WARMUP_GRACE_PERIOD, every concurrency arm — including 32 and 48 — falls back to the 1800-second default.

Why this matters here: the sibling recipe dsv4_fp4_mi355x_sglang_mtp.sh:172-174 implements exactly this override correctly:

if [ "$CONC" -ge 32 ]; then
    export AGENTIC_WARMUP_GRACE_PERIOD=3600
fi

with the comment "Saturation arms carry a larger in-flight working set than the 30-minute default warmup drain allows." glm5.2_fp4_mi355x_sglang_mtp.sh uses the same pattern. And this very PR's perf-changelog.yaml entry explicitly documents the same intent for the ATOM recipe: "Increase the AgentX warmup grace period from the 1800-second default to 3600 seconds at concurrency 32 and 48 ... Keep concurrency 1 through 16 at the default 1800 seconds." The code that's supposed to realize that claim was never written — instead a plausible-looking but nonexistent variable was set.

Why nothing catches it: set -euo pipefail doesn't fail on setting an unused/unread env var — bash has no concept of "this export is consumed by anything," so the line runs successfully and silently does nothing. There's no lint or CI check tying perf-changelog.yaml prose to the actual exported variables in the recipe script, so the mismatch between documented and actual behavior ships clean.

Step-by-step proof:

  1. dsv4_fp4_mi355x_atom_mtp.sh:58 runs export AIPERF_BENCHMARK_GRACE_PERIOD=30.
  2. The script later calls build_replay_cmd "$RESULT_DIR" (benchmark_lib.sh), which at line 2044 builds the CLI arg: REPLAY_CMD+=" --warmup-grace-period ${AGENTIC_WARMUP_GRACE_PERIOD:-1800}".
  3. AGENTIC_WARMUP_GRACE_PERIOD was never exported by this script, so bash substitutes the default 1800, regardless of CONC.
  4. AIPERF_BENCHMARK_GRACE_PERIOD is simply dead — no code path in benchmark_lib.sh or elsewhere reads it (confirmed by grep against all AIPERF_* handling, which processes AIPERF_WARMUP_REQUESTS_PER_LANE, AIPERF_TRACE_IDLE_GAP_CAP_SECONDS, AIPERF_FAILED_REQUEST_THRESHOLD, etc., but never this name).
  5. Net effect: at conc 32 and 48, the replay runs with a 30-minute warmup grace period instead of the intended 60-minute one — exactly the saturation-drain scenario the sibling SGLang recipe's comment identifies as needing the longer window.

Fix: replace the dead export AIPERF_BENCHMARK_GRACE_PERIOD=30 line with the same conditional the sibling script uses:

if [ "$CONC" -ge 32 ]; then
    export AGENTIC_WARMUP_GRACE_PERIOD=3600
fi

placed after CONC is available (it's already required via check_env_vars), or at minimum export AGENTIC_WARMUP_GRACE_PERIOD=3600 unconditionally if simplicity is preferred over the changelog's stated conc-1-16-stays-at-1800 nuance.

Comment thread perf-changelog.yaml Outdated
- agentic-coding
description:
- "Add DeepSeek-V4-Pro FP4 ATOM MTP AgentX on one 8x MI355X node at concurrency 1, 2, 4, 8, 16, 32, and 48, with no KV offload and max-num-seqs set to twice concurrency."
- "Use rocm/atom-dev:nightly_202608181633 (digest sha256:fdc5650f2b6d13c22f1f1b3873ee6dc61278e6b0b90dc24d0f6c55810895032f), FP8 KV/index caches, prefix caching, 32K state checkpoints, 16K batching/prefill chunks, and FULL cudagraph mode."

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.

🟡 The new perf-changelog entry for dsv4-fp4-mi355x-atom-agentic-mtp (line 6205) documents settings that don't match what the PR actually ships: it cites image rocm/atom-dev:nightly_202608181633 (digest sha256:fdc5650f...) while configs/amd-master.yaml sets image: rocm/atom-dev:nightly_202608201032 for the same key, and it claims '32K state checkpoints' while the recipe script sets --state-checkpoint-interval-tokens 8192 (8K). Both are pre-existing values from the original fork PR text that weren'''t updated when this internalization PR bumped the image; please correct the changelog text to match the shipped image/digest and change '''32K''' to '''8K'''.

Extended reasoning...

The new perf-changelog.yaml entry (line 6205, config-key dsv4-fp4-mi355x-atom-agentic-mtp) contains two independent factual mismatches against the code it's supposed to describe.

Image/digest mismatch. The changelog prose reads: "Use rocm/atom-dev:nightly_202608181633 (digest sha256:fdc5650f2b6d13c22f1f1b3873ee6dc61278e6b0b90dc24d0f6c55810895032f)...". But the config entry added in the same diff, configs/amd-master.yaml, sets image: rocm/atom-dev:nightly_202608201032 for that exact config key. These are two different nightly builds — Aug 18 16:33 vs Aug 20 10:32 — introduced in the same PR. Since the config's image: field is what the runner actually pulls and launches, the changelog is documenting a build that was never run for this entry; the digest pin makes it doubly precise and doubly wrong.

Checkpoint-interval mismatch. The same changelog bullet also states "...32K state checkpoints, 16K batching/prefill chunks...". The recipe script benchmarks/single_node/agentic/dsv4_fp4_mi355x_atom_mtp.sh:111 sets --state-checkpoint-interval-tokens 8192, which is 8K, not 32K. The neighboring claim in the same sentence is correct as a sanity check: "16K batching/prefill chunks" correctly maps to --max-num-batched-tokens 16384 and --attn-prefill-chunk-size 16384 in the same script. So this isn't a general misread of the script — only the state-checkpoint value is off by 4x from what's actually configured.

Why the existing code doesn't catch this: perf-changelog.yaml is free-form descriptive prose consumed by humans (and possibly downstream tooling) trying to understand what was validated; nothing cross-checks its text against the YAML config or the shell script flags. CI runs off configs/amd-master.yaml and the .sh script directly, so neither mismatch affects what actually executes or whether the sweep passes.

Proof, step by step:

  1. Diff hunk in configs/amd-master.yaml adds dsv4-fp4-mi355x-atom-agentic-mtp: with image: rocm/atom-dev:nightly_202608201032.
  2. Diff hunk in perf-changelog.yaml adds the entry for the same config-key, with prose citing rocm/atom-dev:nightly_202608181633 (digest sha256:fdc5650f...).
  3. 202608201032 (Aug 20, 10:32) ≠ 202608181633 (Aug 18, 16:33) — two distinct nightly tags, both new in this PR, so neither is a pre-existing key being reused correctly.
  4. In the new script, line 111: --state-checkpoint-interval-tokens 8192. 8192 tokens = 8 * 1024 = 8K, not 32K as the changelog claims.
  5. Cross-check: --max-num-batched-tokens 16384 and --attn-prefill-chunk-size 16384 in the same script both equal 16K, matching the changelog's "16K batching/prefill chunks" claim exactly — confirming the changelog author intended per-flag accuracy, and the 8K/32K entry is simply wrong, not a rounding or unit convention issue.

Likely root cause: the changelog text and pr-link (still pointing to the original fork PR #2668) were carried over verbatim during the internalization, but the config's image tag was bumped to a newer nightly (and/or the checkpoint interval was tuned) without updating the corresponding prose.

Impact and fix: This is documentation-only — the shipped config and script are authoritative for what actually runs, and CI does not read perf-changelog.yaml, so nothing breaks and no behavior changes. However, anyone auditing or trying to reproduce "what was validated" from the changelog would pull the wrong nightly image and misunderstand the checkpoint cadence. Fix by updating line 6205's text to rocm/atom-dev:nightly_202608201032 (with a matching digest, if known) and changing "32K state checkpoints" to "8K state checkpoints".

@github-actions

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

/stage-results 32386715551

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

@cquil11 staged run 32386715551: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-20~r32386715551

This run remains available across future /stage-results requests. Staging the same run ID again updates its staged data. Staging workflow

@cquil11

cquil11 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

/reuse-sweep-run 32386715551

@cquil11
cquil11 merged commit c2b5803 into main Aug 20, 2026
48 checks passed
@cquil11
cquil11 deleted the amd/agentx_dsv4_atom_0821 branch August 20, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentx AgentX benchmarks, recipes, and infrastructure AMD full-sweep-enabled

Projects

Development

Successfully merging this pull request may close these issues.

2 participants