[AMD][MI355X] Add DSv4 FP4 agentic MTP recipe with DPA tuning#2254
[AMD][MI355X] Add DSv4 FP4 agentic MTP recipe with DPA tuning#2254seungrokj wants to merge 12 commits 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 关于重新运行失败任务的文档 |
|
|
||
| - 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" |
There was a problem hiding this comment.
🔴 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) callsChangelogEntry.model_validate(entry)inside a try/except that re-raises asChangelogValidationErroron failure.utils/process_changelog.py:158does the same viaChangelogEntry.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:
utils/validate_perf_changelog.pyloadsperf-changelog.yamlas a list of dicts, one per- config-keys: ...block.- It iterates entries and calls
ChangelogEntry.model_validate(entry)for each, including the new dict{"config-keys": ["dsv4-fp4-mi355x-sglang-agentic-mtp"], "description": [...]}(nopr-linkkey). - Pydantic checks required fields on
ChangelogEntry;pr_link(aliasedpr-link) has no default and is absent from the input dict, somodel_validateraisespydantic.ValidationError: 1 validation error for ChangelogEntry / pr-link / Field required. - The surrounding try/except in
validate_perf_changelog.pycatches this and raisesChangelogValidationError, failing the changelog check for the entire file (not just the new entry). - 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.
| 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" |
There was a problem hiding this comment.
🔴 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"
fiEvery 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:
- A maintainer runs (or a PR is labeled for) an evals-only/all-evals sweep that includes this new recipe.
generate_sweep_configs.pycallsmark_eval_entries(..., include_agentic=True), which finds the max-conc entry fordsv4-fp4-mi355x-sglang-agentic-mtp's agentic-coding scenario and setsRUN_EVAL=True, thenEVAL_ONLY=truefor that job.- The job is dispatched to
dsv4_fp4_mi355x_sglang_mtp.shwithEVAL_ONLY=truein its environment. - 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 callsbuild_replay_cmd+run_agentic_replay_and_write_outputs. - 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"
fiThis 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.
| 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] } |
There was a problem hiding this comment.
🟡 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:
- Before this PR:
dsv4-fp4-mi355x-sglang-agentic-hicache:appears once, at (pre-PR) line ~2465. - 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 newdsv4-fp4-mi355x-sglang-agentic-mtp:block. - Post-PR,
grep -n '^dsv4-fp4-mi355x-sglang-agentic-hicache:' configs/amd-master.yamlreturns two hits (lines 2452 and 2468 in the final file). - Running the file through
yaml.safe_loadcollapses 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.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29548395719 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29548395719 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29555826772 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29562088440 |
- 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>
d895957 to
4ddd584
Compare
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>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29635054984 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29635071153 |
…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>
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>
Summary
dsv4-fp4-mi355x-sglang-agentic-mtpconfig (separate from hicache)dsv4_fp4_mi355x_sglang_mtp.shwith EAGLE spec decoding (3 steps, topk 1, 4 draft tokens)--enable-prefill-delayer,GPU_MAX_HW_QUEUES=5, chunked prefill8192*TPMAX_RUNNING_REQUESTS = 2*CONC,CUDA_GRAPH_MAX_BS = CONC(cap 128)MEM_FRACTION_STATICreduced by 0.10 when spec decoding is activelmsysorg/sglang-rocm:v0.5.15.post1-rocm720-mi35x-20260714Test plan
🤖 Generated with Claude Code