Skip to content

[AMD][MI355X] Add DSv4 FP4 agentic MTP recipe with DPA tuning#2254

Open
seungrokj wants to merge 12 commits into
mainfrom
amd/agentx_dsv4_sgl_mtp_0717
Open

[AMD][MI355X] Add DSv4 FP4 agentic MTP recipe with DPA tuning#2254
seungrokj wants to merge 12 commits into
mainfrom
amd/agentx_dsv4_sgl_mtp_0717

Conversation

@seungrokj

Copy link
Copy Markdown
Collaborator

Summary

  • Add new dsv4-fp4-mi355x-sglang-agentic-mtp config (separate from hicache)
  • New benchmark script dsv4_fp4_mi355x_sglang_mtp.sh with EAGLE spec decoding (3 steps, topk 1, 4 draft tokens)
  • DPA tuning: --enable-prefill-delayer, GPU_MAX_HW_QUEUES=5, chunked prefill 8192*TP
  • MAX_RUNNING_REQUESTS = 2*CONC, CUDA_GRAPH_MAX_BS = CONC (cap 128)
  • MEM_FRACTION_STATIC reduced by 0.10 when spec decoding is active
  • Sweep TP8 conc [4,8] non-DPA + conc [16,32,48,52] DPA
  • Image: lmsysorg/sglang-rocm:v0.5.15.post1-rocm720-mi35x-20260714

Test plan

  • Full sweep on MI355X cluster with non-DPA conc [4,8] and DPA conc [16,32,48,52]
  • Verify no HSA out-of-resources crashes during warmup
  • Confirm prefill delayer improves decode throughput at high concurrency

🤖 Generated with Claude Code

@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 full-sweep-fail-fast agentx AgentX benchmarks, recipes, and infrastructure labels Jul 17, 2026
Comment thread perf-changelog.yaml Outdated
Comment on lines +4921 to +4931

- config-keys:
- dsv4-fp4-mi355x-sglang-agentic-mtp
description:
- "Add DSv4 FP4 MI355X SGLang agentic MTP recipe (separate from hicache config)"
- "Image: lmsysorg/sglang-rocm:v0.5.15.post1-rocm720-mi35x-20260714"
- "EAGLE speculative decoding: 3 steps, topk 1, 4 draft tokens"
- "DPA: --enable-prefill-delayer, GPU_MAX_HW_QUEUES=5, chunked prefill 8192*TP"
- "MAX_RUNNING_REQUESTS = 2*CONC, CUDA_GRAPH_MAX_BS = CONC (cap 128)"
- "MEM_FRACTION_STATIC reduced by 0.10 when spec decoding active"
- "Sweep TP8 conc [4,8] non-DPA + conc [16,32,48,52] DPA"

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.yaml entry for dsv4-fp4-mi355x-sglang-agentic-mtp is missing the required pr-link field, which every other entry in the file includes. Since ChangelogEntry in utils/matrix_logic/validation.py has no default for pr-link and forbids extra fields, this will fail Pydantic validation and block changelog processing / the sweep for this PR — please add pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2254.

Extended reasoning...

What the bug is: The changelog entry appended at the end of perf-changelog.yaml (lines 4921-4931) for config-keys: [dsv4-fp4-mi355x-sglang-agentic-mtp] has a config-keys list and a description list, but no pr-link key. Every other entry in the file (615 of 616 config-keys blocks) includes a pr-link, including the immediately preceding entry at line 4920 (pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2241).

Why this breaks: ChangelogEntry in utils/matrix_logic/validation.py:857-869 is:

class ChangelogEntry(BaseModel):
    model_config = ConfigDict(extra="forbid", populate_by_name=True)

    config_keys: list[str] = Field(alias="config-keys", min_length=1)
    description: list[str] = Field(min_length=1)
    pr_link: str = Field(alias="pr-link")
    ...

pr_link has no default=, so under Pydantic it is a required field. Nothing in this model treats it as optional.

Code path that triggers it: Both entry points that consume perf-changelog.yaml iterate every entry and validate it against ChangelogEntry:

  • utils/validate_perf_changelog.py (_parse_and_validate, ~line 129) calls ChangelogEntry.model_validate(entry) inside a try/except that re-raises as ChangelogValidationError on failure.
  • utils/process_changelog.py:158 does the same via ChangelogEntry.model_validate(entry_data).

Because the new entry is missing pr-link, model_validate will raise a "Field required" error for that field, which validate_perf_changelog.py turns into a ChangelogValidationError for the whole file. Per AGENTS.md, changelog validation runs before sweep setup, so this hard-fails changelog validation and blocks the sweep/PR from proceeding — it isn't a lint-only nuisance.

Why nothing else catches it: There's no partial-validation fallback — the loop validates every entry and aborts on the first failure, so this one missing field is sufficient to break the whole file's validation, not just this entry.

Step-by-step proof:

  1. utils/validate_perf_changelog.py loads perf-changelog.yaml as a list of dicts, one per - config-keys: ... block.
  2. It iterates entries and calls ChangelogEntry.model_validate(entry) for each, including the new dict {"config-keys": ["dsv4-fp4-mi355x-sglang-agentic-mtp"], "description": [...]} (no pr-link key).
  3. Pydantic checks required fields on ChangelogEntry; pr_link (aliased pr-link) has no default and is absent from the input dict, so model_validate raises pydantic.ValidationError: 1 validation error for ChangelogEntry / pr-link / Field required.
  4. The surrounding try/except in validate_perf_changelog.py catches this and raises ChangelogValidationError, failing the changelog check for the entire file (not just the new entry).
  5. Because changelog validation happens before sweep setup, the sweep/PR check fails deterministically — this isn't flaky or environment-dependent.

Fix: Add a pr-link line to the new entry, e.g. pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2254, matching every other entry in the file.

Comment on lines +199 to +205
fi

# ---- Run benchmark ----------------------------------------------------------
build_replay_cmd "$RESULT_DIR"
REPLAY_CMD+=" --server-metrics http://localhost:$SGLANG_BACKEND_PORT/metrics"

run_agentic_replay_and_write_outputs "$RESULT_DIR"

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.

🔴 New script dsv4_fp4_mi355x_sglang_mtp.sh unconditionally runs the throughput replay (build_replay_cmd + run_agentic_replay_and_write_outputs) and never checks EVAL_ONLY, unlike its sibling dsv4_fp4_mi355x_sglang.sh (and all other agentic single-node scripts), which gate this behind if [ "${EVAL_ONLY}" = "true" ]; then run_eval ...; else <replay>; fi. Since the new dsv4-fp4-mi355x-sglang-agentic-mtp config defines an agentic-coding scenario, mark_eval_entries can mark it EVAL_ONLY=true during evals-only/all-evals sweeps, and this script will silently ignore that flag, run the throughput benchmark instead of the eval, and waste the GPU run producing no eval artifacts.

Extended reasoning...

The bug: benchmarks/single_node/agentic/dsv4_fp4_mi355x_sglang_mtp.sh ends its script (lines 199-205) with an unconditional:

build_replay_cmd "$RESULT_DIR"
REPLAY_CMD+=" --server-metrics http://localhost:$SGLANG_BACKEND_PORT/metrics"
run_agentic_replay_and_write_outputs "$RESULT_DIR"

with no check of EVAL_ONLY anywhere in the file. Compare this to the sibling script it was copied from, dsv4_fp4_mi355x_sglang.sh, whose tail (verified above) is:

if [ "${EVAL_ONLY}" = "true" ]; then
    run_eval --port "$PORT"
else
    build_replay_cmd "$RESULT_DIR"
    REPLAY_CMD+=" --server-metrics http://localhost:$SGLANG_BACKEND_PORT/metrics"
    run_agentic_replay_and_write_outputs "$RESULT_DIR"
fi

Every other agentic single-node script in the repo (23 of them, confirmed via grep for EVAL_ONLY) has this same gate. The new MTP script is the sole exception, meaning the copy dropped the conditional when adapting the file for MTP/EAGLE decoding.

Why the gate matters and is reachable: This isn't decorative. generate_sweep_configs.py has a mark_eval_entries(matrix_values, include_agentic=...) step (called with include_agentic = args.evals_only or args.all_evals) that, for single-node agentic-coding scenario entries, marks the highest-concurrency entry per (model, runner, framework, precision) with RUN_EVAL=true, and a subsequent step sets EVAL_ONLY=true on those entries when the sweep is run with the evals-only/all-evals label. benchmark_lib.sh's run_eval() has agentic-specific handling (a scenario_is_agentic branch) that runs an accuracy eval (e.g. SWE-bench) instead of the throughput replay — this is the entire point of the EVAL_ONLY contract.

The new config dsv4-fp4-mi355x-sglang-agentic-mtp (added in configs/amd-master.yaml) defines exactly an agentic-coding scenario, single-node (multinode: false), so it is a valid candidate for mark_eval_entries to select and flag EVAL_ONLY=true during an evals-only/all-evals sweep run — a supported, documented workflow already used elsewhere in this repo (e.g. via PR labels per .github/workflows/run-sweep.yml).

Concrete walkthrough of the failure:

  1. A maintainer runs (or a PR is labeled for) an evals-only/all-evals sweep that includes this new recipe.
  2. generate_sweep_configs.py calls mark_eval_entries(..., include_agentic=True), which finds the max-conc entry for dsv4-fp4-mi355x-sglang-agentic-mtp's agentic-coding scenario and sets RUN_EVAL=True, then EVAL_ONLY=true for that job.
  3. The job is dispatched to dsv4_fp4_mi355x_sglang_mtp.sh with EVAL_ONLY=true in its environment.
  4. The script starts the SGLang server (with its EAGLE spec-decoding/DPA settings, unrelated to the bug) and then reaches the tail of the script, which — because there is no if [ "${EVAL_ONLY}" = "true" ] check — unconditionally calls build_replay_cmd + run_agentic_replay_and_write_outputs.
  5. The intended run_eval --port "$PORT" call (which would trigger the SWE-bench/agentic accuracy eval) never executes. The job instead runs a full aiperf throughput trace-replay, burning the entire GPU allocation, and produces no eval scores or artifacts where the CI/sweep pipeline expects them.

Why nothing else prevents this: check_env_vars at the top of the script validates that required vars are set, but EVAL_ONLY isn't in its required list and there's no downstream validation that would catch a silently-ignored flag — the script just runs to the end unconditionally.

Fix: wrap the tail in the same gate as the sibling script:

if [ "${EVAL_ONLY}" = "true" ]; then
    run_eval --port "$PORT"
else
    build_replay_cmd "$RESULT_DIR"
    REPLAY_CMD+=" --server-metrics http://localhost:$SGLANG_BACKEND_PORT/metrics"
    run_agentic_replay_and_write_outputs "$RESULT_DIR"
fi

This only affects the opt-in evals-only/all-evals path — the default full-sweep-fail-fast throughput sweep (the PR's stated test plan) is unaffected since include_agentic defaults to false there — but any evals run against this recipe will silently produce no eval output until fixed.

Comment thread configs/amd-master.yaml
Comment on lines +2468 to +2482
dsv4-fp4-mi355x-sglang-agentic-hicache:
image: lmsysorg/sglang-rocm:v0.5.14-rocm720-mi35x-20260710
model: deepseek-ai/DeepSeek-V4-Pro
model-prefix: dsv4
runner: cluster:mi355x-amds
precision: fp4
framework: sglang
multinode: false
scenarios:
agentic-coding:
- dram-utilization: 0.80
search-space:
- { tp: 8, kv-offloading: none, conc-list: [1, 2, 4, 8] }
- { tp: 8, dp-attn: true, kv-offloading: none, conc-list: [16, 32, 48, 64] }
- { tp: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: hicache }, conc-list: [64] }

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.

🟡 This PR accidentally leaves a second, byte-for-byte identical dsv4-fp4-mi355x-sglang-agentic-hicache: block in configs/amd-master.yaml (lines 2468-2482), duplicating the pre-existing entry directly above it. It's a copy-paste artifact from using the hicache block as a template for the new dsv4-fp4-mi355x-sglang-agentic-mtp entry — please delete the duplicate block.

Extended reasoning...

The diff adds a second top-level dsv4-fp4-mi355x-sglang-agentic-hicache: mapping key immediately below the pre-existing one, right before the new dsv4-fp4-mi355x-sglang-agentic-mtp: entry that this PR is actually meant to introduce. In the resulting file this shows up as two adjacent, byte-for-byte identical blocks: the pre-existing entry at lines 2452-2466, followed by the newly-added duplicate at lines 2468-2482.

Code path: configs/amd-master.yaml is loaded via yaml.safe_load in load_config_files() (utils/matrix_logic/validation.py:919). PyYAML's default loader does not error on duplicate mapping keys within a single document — it silently keeps only the last occurrence. Since both dsv4-fp4-mi355x-sglang-agentic-hicache blocks are identical, the "last one wins" collapse produces the exact same effective config as if the duplicate had never been added, so there is no behavioral difference today.

Why the repo's own duplicate-key guard doesn't catch it: load_config_files() does have a duplicate-key check at validation.py:930-936, but it only detects a key being duplicated across different config files (it intersects config_data.keys() from the current file against all_config_data.keys() accumulated from previously-loaded files). By the time that check runs, yaml.safe_load has already silently collapsed the intra-file duplicate down to one key, so the duplication within amd-master.yaml itself is invisible to it. There's also no .yamllint config in the repo that would otherwise flag duplicate keys at CI time.

Impact: Purely cosmetic/dead config today, since the two blocks are identical and the parser deterministically resolves to one value. However, this is exactly the kind of artifact that causes real confusion later: if a future PR edits one of the two blocks (e.g. bumping the image tag or tweaking the search space) without noticing the sibling duplicate, the edit could silently have no effect (if the untouched copy happens to load last) or silently take effect while looking like a no-op in a diff review. It's clearly unintentional — the PR description says it's adding only the new -mtp entry, and the mechanism is obviously that the hicache block was copied as a scaffold for the new entry and the copy of the original block was never deleted.

Proof / walkthrough:

  1. Before this PR: dsv4-fp4-mi355x-sglang-agentic-hicache: appears once, at (pre-PR) line ~2465.
  2. This PR's diff inserts 32 new lines directly after it, starting with dsv4-fp4-mi355x-sglang-agentic-hicache: again (identical image, model, scenarios, search-space), followed by the new dsv4-fp4-mi355x-sglang-agentic-mtp: block.
  3. Post-PR, grep -n '^dsv4-fp4-mi355x-sglang-agentic-hicache:' configs/amd-master.yaml returns two hits (lines 2452 and 2468 in the final file).
  4. Running the file through yaml.safe_load collapses these to a single key with the (identical) value of the second occurrence — no functional change today, but a landmine for future edits and a source of reviewer confusion.

Fix: Delete the duplicated dsv4-fp4-mi355x-sglang-agentic-hicache: block introduced by this PR (lines 2468-2482), leaving the original entry and the new dsv4-fp4-mi355x-sglang-agentic-mtp: entry intact.

@github-actions

Copy link
Copy Markdown
Contributor

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

seungrokj and others added 6 commits July 18, 2026 15:49
- New config dsv4-fp4-mi355x-sglang-agentic-mtp (separate from hicache)
- Image: lmsysorg/sglang-rocm:v0.5.15.post1-rocm720-mi35x-20260714
- EAGLE spec decoding: 3 steps, topk 1, 4 draft tokens
- DPA: prefill delayer, GPU_MAX_HW_QUEUES=5, chunked prefill 8192*TP
- MAX_RUNNING_REQUESTS=2*CONC, CUDA_GRAPH_MAX_BS=CONC (cap 128)
- MEM_FRACTION_STATIC reduced by 0.10 for spec decoding headroom
- Sweep TP8 conc [4,8] non-DPA + conc [16,32,48,52] DPA

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…entry

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ChangelogMatrixEntry.evals only accepted SingleNodeMatrixEntry, causing
agentic-coding configs to fail validation with "Extra inputs not permitted"
for fields like kv-offloading, duration, scenario-type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The eval pipeline injects run-eval and eval-only into agentic entries,
but SingleNodeAgenticMatrixEntry and MultiNodeAgenticMatrixEntry had
extra='forbid' without those fields, causing validation to reject them.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set MEM_FRACTION_STATIC=0.75 under DPA for speculative-decode headroom,
double chunked-prefill to 16K/scheduler for agentic prefill tails,
set MAX_RUNNING_REQUESTS=CONC, and partition CUDA_GRAPH_MAX_BS across
DP ranks.

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

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

seungrokj and others added 2 commits July 19, 2026 01:13
…o c48

Disable the 0.75 mem_fraction_static override that caused a 6x KV cache
reduction (5.1M → 873K tokens), falling back to the default 0.90.
Narrow the sweep to DPA c48 only for a focused retest.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the per-DP-rank division of CUDA_GRAPH_MAX_BS — keep it at CONC
so decode batches up to the concurrency level stay on the CUDA graph
fast path (was dropping from 52 to 7 with TP8).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
seungrokj and others added 3 commits July 19, 2026 01:22
Revert MAX_RUNNING_REQUESTS from CONC back to 2*CONC to match the
old (good) run configuration that achieved 5,985 completed records.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert per-scheduler chunked prefill from 16K to 8K to match the old
run configuration (8192*TP = 65536 total vs 16384*TP = 131072).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Re-add GPU_MAX_HW_QUEUES=5 under DPA to enable compute/memory op
overlap on ROCm, matching the old run configuration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant