Skip to content

perf(agentx): add DeepSeek V4 FP8 MTP frontier on MI325X - #2541

Open
cquil11 wants to merge 20 commits into
mainfrom
agent/dsv4-mi325x-agentic-mtp
Open

perf(agentx): add DeepSeek V4 FP8 MTP frontier on MI325X#2541
cquil11 wants to merge 20 commits into
mainfrom
agent/dsv4-mi325x-agentic-mtp

Conversation

@cquil11

@cquil11 cquil11 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add the first MI325X DeepSeek-V4-Pro FP8 AgentX frontier on stable vLLM v0.26.0
  • keep MTP enabled for TP8, TP8+EP8, and DP8+EP8 topologies
  • sample a dense 34-point single-node Pareto search up to concurrency 80
  • pass explicit vLLM server-metrics and request cache accounting to AIPerf

Search space

  • TP8/EP1: c1,2,4,6,8,10,12,14,16,18,20,22,24,28,32
  • TP8/EP8: c4,8,12,16,20,24,28,32,36,40
  • DP8/EP8: c16,24,32,40,48,56,64,72,80

All points are one-node/GPU-resident, use FP8 KV, MTP3 with golden synthetic acceptance length 2.49, FULL_DECODE_ONLY graph capture, block size 256, and an 8192-token scheduler budget. DP8 uses session-aware vLLM routing across all eight ranks.

Validation

  • config generator expands exactly 34 intended throughput points
  • 224/224 matrix tests pass
  • benchmark script passes Bash syntax validation
  • perf changelog and diff checks pass
  • stable v0.26.0 image, DeepSeek-V4 FP8 target+MTP loading, graph capture, service health, one explicit metrics endpoint, and DP8 session distribution verified on MI325X
  • representative direct profiles and final metrics exports
  • official agentx-fast refinement sweep
  • official full sweep

Draft while cluster validation is in progress.

@github-actions

github-actions Bot commented Aug 9, 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 关于重新运行失败任务的文档

@cquil11
cquil11 marked this pull request as ready for review August 9, 2026 23:19
@cquil11
cquil11 requested a review from a team August 9, 2026 23:19
@cquil11 cquil11 added the agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable label Aug 9, 2026
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cquil11's task in 4m 21s —— View job


Review of PR #2541

LGTM - no blocking issues found

Verified against repo conventions and found nothing to flag:

  • configs/amd-master.yaml modified together with perf-changelog.yaml, and the new changelog entry is appended at the end of the file
  • Image vllm/vllm-openai-rocm:v0.26.0 is a public Docker Hub reference, consistent with existing MI325X vLLM entries
  • Model prefix dsv4 is an established prefix already used throughout both master configs
  • --enable-expert-parallel is correctly gated on EP_SIZE > 1 (not hardcoded), and the DP-attention arm enforces EP_SIZE == TP matching the config (ep: 8, tp: 8)
  • Server launch command is formatted one argument per line (array form)
  • Script name agentic/dsv4_fp8_mi325x_mtp.sh matches the launch_mi325x-amds.sh routing convention (${EXP_NAME%%_*}_${PRECISION}_mi325x${SPEC_SUFFIX}.sh), so no launcher change is needed
  • Agentic replay goes through build_replay_cmd (chat-completions endpoint, template applied server-side), so the --use-chat-template MTP requirement for benchmark_client doesn't apply — same as the sibling dsv4_fp4_mi355x_vllm_mtp.sh
  • --quantization deepseek_v4_fp8, MTP spec-config with golden synthetic acceptance, vllm-router flags, and cleanup traps all mirror existing validated MI325X/MI355X recipes (stop_background_process_tree safely handles the empty ROUTER_PID in pure-TP runs)
  • Search space expands to 15 + 10 + 9 = 34 points as stated; inline router: key in the DP arm has existing precedent (amd-master.yaml:1248)
  • CI: 0 failures so far (static PR checks green, benchmark sweep queued)

Note: I couldn't execute pytest utils/matrix_logic or bash -n locally due to sandbox tool restrictions in this run, but the equivalent static checks pass in CI and the author reports 224/224 matrix tests passing.

Comment on lines +108 to +112
--gpu-memory-utilization 0.9
--block-size 256
--max-num-batched-tokens 8192
--max-num-seqs "$MAX_NUM_SEQS"
--compilation-config '{"mode":3,"cudagraph_mode":"FULL_DECODE_ONLY"}'

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.

🔴 dsv4_fp8_mi325x_mtp.sh combines cudagraph_mode FULL_DECODE_ONLY with MTP (num_speculative_tokens=3) but never sets --max-cudagraph-capture-size or an explicit cudagraph_capture_sizes list, unlike the two sibling DSv4-MTP scripts (dsv4_fp4_b200_vllm_mtp.sh, dsv4_fp4_b300_vllm_mtp.sh) that use the identical combination. Because vLLM cudagraph capture sizes are counted in tokens, a decode batch of S sequences verifies S*(1+N) tokens, so the default sizing ladder collapses to only ~MAX_NUM_SEQS/(1+N) sequences of graph coverage; every decode batch above that silently falls back to eager execution — degrading throughput precisely at the high-concurrency points (up to conc80, MAX_NUM_SEQS=160) this sweep is built to measure. Fix by adding --max-cudagraph-capture-size (or an explicit cudagraph_capture_sizes list) sized to MAX_NUM_SEQS*(1+NUM_SPEC_TOKENS), matching the two sibling scripts.

Extended reasoning...

The bug: dsv4_fp8_mi325x_mtp.sh line 112 sets --compilation-config '{"mode":3,"cudagraph_mode":"FULL_DECODE_ONLY"}' and combines it with MTP speculative decoding (num_speculative_tokens=3, lines 77-82). MAX_NUM_SEQS is set to 2 * CONC (line 66), reaching 160 at the highest sampled concurrency (conc80, DP8+EP8). Nowhere in the script is --max-cudagraph-capture-size or an explicit cudagraph_capture_sizes list passed.

Why this matters: vLLM's cudagraph capture-size ladder for FULL_DECODE_ONLY mode is expressed in tokens, not sequences. With MTP, a decode step for a batch of S sequences verifies S*(1+N) tokens (1 accepted/bonus token plus N speculative tokens per sequence). vLLM's adjust_cudagraph_sizes_for_spec_decode rounds the default (sequence-derived) capture-size ladder up to multiples of (1+N) and dedups it. Without an explicit override, this collapses the effective decode-graph coverage down to roughly MAX_NUM_SEQS/(1+N) sequences — for this recipe, only ~40 sequences of graph coverage even though decode batches of up to 160 sequences are expected at conc80. Any decode batch above the collapsed threshold silently falls back to eager execution instead of a captured CUDA graph. There is no error or warning — the benchmark simply runs slower at exactly the points it's meant to characterize.

Why existing code doesn't prevent it: Two sibling scripts in the same PR family use the identical FULL_DECODE_ONLY + MTP(N=3) combination and both explicitly guard against this:

  • dsv4_fp4_b200_vllm_mtp.sh (lines 239-255) computes MAX_CUDAGRAPH_CAPTURE_SIZE=MAX_NUM_SEQS*(1+NUM_SPEC_TOKENS) and passes it via --max-cudagraph-capture-size, with a comment explaining the tokens-vs-sequences mechanism verbatim.
  • dsv4_fp4_b300_vllm_mtp.sh (lines 261-286) builds an explicit cudagraph_capture_sizes list containing every multiple of (1+N) up to MAX_NUM_SEQS*(1+N), citing adjust_cudagraph_sizes_for_spec_decode by name.

The new MI325X script omits this handling entirely. The mode:3 vs the siblings' mode:0 difference is the compilation mode, unrelated to cudagraph_mode (which is FULL_DECODE_ONLY in all three) or to the spec-decode capture-size adjustment, so it does not sidestep the issue.

Step-by-step proof:

  1. At conc80 (DP8+EP8), MAX_NUM_SEQS = 2*80 = 160.
  2. NUM_SPEC_TOKENS = 3, so each decode step for a full batch verifies 160*(1+3) = 640 tokens.
  3. Without an explicit override, vLLM derives its default capture-size ladder from max_num_seqs in units that get token-adjusted and rounded to multiples of (1+3)=4, then deduped — collapsing effective coverage to roughly 160/4 = 40 sequences.
  4. Any decode batch with more than ~40 concurrent sequences (i.e., most of the batches sampled at conc>=20-24 and definitely all batches near conc80) has no matching captured graph and falls back to eager execution.
  5. Eager decode is measurably slower than a captured CUDA graph, so AIPerf's measured throughput at the high-concurrency sweep points is understated relative to what the recipe (and its siblings) intend to measure — silently, with no visible error in logs.

Impact: This is a benchmarking recipe whose entire purpose is to produce an accurate throughput frontier across concurrency 1-80. Because the failure mode kicks in exactly at the higher end of that range (conc>=~20-24 through conc80), it corrupts the highest-value data points in the sweep — the DP8+EP8 throughput frontier — without any test or log signal that something is wrong.

Fix: Add MAX_CUDAGRAPH_CAPTURE_SIZE=0 and pass it via --max-cudagraph-capture-size (or build an explicit cudagraph_capture_sizes list of multiples of (1+NUM_SPEC_TOKENS) up to that value), mirroring dsv4_fp4_b200_vllm_mtp.sh/dsv4_fp4_b300_vllm_mtp.sh.

Comment thread perf-changelog.yaml
Comment on lines +5712 to +5719

- config-keys:
- dsv4-fp8-mi325x-vllm-agentic-mtp
scenario-type:
- agentic-coding
description:
- "Add the first DeepSeek-V4-Pro FP8 vLLM AgentX submission on MI325X, with MTP enabled for every published point and the committed three-token golden synthetic acceptance length of 2.49; eval-only runs retain real target verification."
- "Use one 8xMI325X node: the roughly 1.05 TB FP8-dequantized checkpoint fits in the node's 2 TB HBM at TP8, so multi-node execution would add communication without solving a capacity constraint; TP4 is excluded because its per-rank model footprint is not fit-safe."

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's title and description are English-only, but AGENTS.md mandates that every PR title use the <English title> / <中文标题> format and that the body include a Chinese section (e.g. ## 中文说明) mirroring the summary. Please add the Chinese title suffix and a 中文说明 section before merging.

Extended reasoning...

AGENTS.md (repo root, referenced by CLAUDE.md) states explicitly and in bold:

PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format: <English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a ## 中文说明 section mirroring the summary...).

This is a mandatory, every-PR convention, not a suggestion — it applies "to every PR and every issue, matching the bilingual docs rule in Code Conventions." The only documented exceptions are the CODEOWNER sign-off template (which must stay English-verbatim so the sign-off verifier bot can match its exact phrase) and bot-generated comments following their own workflow templates. Neither exception applies here since the PR title and description are author-written content.

The violation: This PR's title is perf(agentx): add DeepSeek V4 FP8 MTP frontier on MI325X — English only, with no / <中文标题> suffix. The description (Summary / Search space / Validation sections) is entirely in English with no ## 中文说明 or equivalent Chinese translation section anywhere in the body.

Why nothing else catches this: there's no CI check enforcing bilingual PR content in this repo (the only automated gate mentioned in the timeline is the CODEOWNER sign-off phrase bot), so this convention is currently enforced only by human/agent reviewers reading AGENTS.md, which this PR did not follow.

Proof (step-by-step):

  1. Open AGENTS.md at the repo root — line 7 states the bilingual requirement in bold with "must."
  2. Read the PR title: perf(agentx): add DeepSeek V4 FP8 MTP frontier on MI325X. Per the required format <English title> / <中文标题>, this should end with a / <中文标题> segment — it does not.
  3. Read the PR description body: it has ## Summary, ## Search space, ## Validation sections, all in English. There is no ## 中文说明 (or similarly named) section anywhere.
  4. Therefore the PR fails the explicit, mandatory bilingual convention.

Fix: rename the PR title to perf(agentx): add DeepSeek V4 FP8 MTP frontier on MI325X / 在 MI325X 上新增 DeepSeek V4 FP8 MTP 前沿配置 (or similar), and append a ## 中文说明 section to the description that mirrors the Summary/Search space/Validation content in natural technical Chinese, following the terminology guidance elsewhere in AGENTS.md (keep model names, SKUs, framework names, and CLI flags in English).

This is a real, currently-checkable, trivially-fixable convention violation, but it does not affect the benchmark configuration, the launch script, or any execution path — nothing breaks, no incorrect data is produced, and the impact is purely documentation/process compliance. Per the review rubric, this doesn't rise to the level of a merge-blocking correctness issue, so it's best treated as a nit for the author to clean up.

Comment on lines +1 to +6
#!/usr/bin/env bash
set -euo pipefail

# DeepSeek-V4-Pro FP8 AgentX replay on one 8xMI325X node. The checkpoint is
# dequantized to FP8 because gfx942 has no native MXFP4 support. Both supported
# one-node layouts retain MTP: TP8 for the latency frontier, and DP8+EP8 for the

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 header comment (lines 4-6) says only two layouts are supported ('TP8 for the latency frontier, and DP8+EP8 for the high-throughput frontier'), but the script and amd-master.yaml actually define three distinct bands: TP8/EP1, TP8/EP8 (dp-attn false), and DP8/EP8 (dp-attn true). The middle TP8+EP8 band is a real, separately-tuned topology that the comment omits, which will mislead future readers about the script's search space.

Extended reasoning...

The file header at benchmarks/single_node/agentic/dsv4_fp8_mi325x_mtp.sh lines 4-6 states: Both supported one-node layouts retain MTP: TP8 for the latency frontier, and DP8+EP8 for the high-throughput frontier. This phrasing hard-counts exactly two topologies.

However, both the script's own branching logic and the configs/amd-master.yaml search-space entry for dsv4-fp8-mi325x-vllm-agentic-mtp define three distinct bands:

  1. tp: 8, ep: 1, dp-attn: false — pure tensor-parallel, no expert-parallel (the 'TP8' latency band)
  2. tp: 8, ep: 8, dp-attn: false — tensor-parallel attention with expert-parallel MoE
  3. tp: 8, ep: 8, dp-attn: true — data-parallel attention with expert-parallel MoE (the 'DP8+EP8' throughput band, routed through vllm-router)

Band 2 is a genuinely distinct configuration in the script: it sets EP_ARGS=(--enable-expert-parallel) because EP_SIZE > 1, but keeps PARALLEL_ARGS=(--tensor-parallel-size "$TP" --data-parallel-size 1) and skips the vLLM router entirely, since DP_ATTENTION is still false. It doesn't fit either label the header uses — it isn't the EP1 'TP8' band, and it isn't the DP-attention 'DP8+EP8' band. The PR description itself independently lists all three bands (TP8/EP1, TP8/EP8, DP8/EP8) with separate concurrency lists, confirming the author treats them as three separately-swept configurations, not two.

Why the existing code doesn't catch this: it's a comment, not logic, so nothing enforces consistency between the header prose and the actual search-space list it describes. The only 'check' is a human reading both together.

Impact is limited to documentation quality — the script executes correctly regardless of what the header says, since the header text isn't parsed or used by any tooling. But a future engineer skimming this file to understand 'what topologies does this recipe cover' before extending it (e.g. adding a fourth band, or reusing this script for another SKU) would incorrectly believe there are only two supported layouts and could miss that a TP8+EP8/dp-attn-false band exists and has its own tuned concurrency curve (c4-c40) distinct from both the EP1 TP8 band (c1-c32) and the DP8+EP8 band (c16-c80).

Suggested fix: reword the header to enumerate all three bands, e.g.: Three one-node layouts retain MTP: TP8/EP1 for the latency frontier, TP8/EP8 for a mid-range throughput band, and DP8/EP8 (DP-attention) for the high-throughput frontier.

Step-by-step proof: (1) Read the header claim — 'Both ... TP8 ... and DP8+EP8' → implies exactly 2 layouts. (2) Read configs/amd-master.yaml's dsv4-fp8-mi325x-vllm-agentic-mtp.scenarios.agentic-coding[0].search-space → it has 3 list entries with different ep/dp-attn combinations. (3) Cross-check against the script: if (( EP_SIZE > 1 )); then EP_ARGS=(--enable-expert-parallel); fi is a branch entirely orthogonal to the DP_ATTENTION branch that selects PARALLEL_ARGS/enables the router — so EP_SIZE=8 with DP_ATTENTION=false (row 2) is a real, reachable, and distinct code path from both row 1 (EP_SIZE=1) and row 3 (DP_ATTENTION=true). (4) Therefore the header's count of 2 is factually short by one band.

Comment on lines +13 to +16
if [[ "$KV_OFFLOADING" != "none" ]]; then
echo "ERROR: DeepSeek-V4 MTP on MI325X currently supports GPU-resident KV only" >&2
exit 1
fi

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.

🟡 Lines 13-16 hand-roll the KV-offloading guard instead of calling benchmark_lib.sh's require_agentic_kv_offload_none, which every other none-only agentic vLLM recipe uses. This is a pre-existing pattern issue introduced by this new file (not a regression of shared code), but it should be fixed before merge: replace the block with a call to the shared helper so a stray KV_OFFLOAD_BACKEND env var fails fast here too, matching dsv4_fp4_b300_vllm_mtp.sh, dsv4_fp4_b200_vllm_mtp.sh, kimik3_fp4_b300_vllm_mtp.sh, and kimik3_fp4_b300_vllm.sh.

Extended reasoning...

What the bug is

benchmarks/single_node/agentic/dsv4_fp8_mi325x_mtp.sh (lines 13-16) hand-rolls its "GPU-resident KV only" guard instead of calling the shared require_agentic_kv_offload_none helper from benchmark_lib.sh:

if [[ "$KV_OFFLOADING" != "none" ]]; then
    echo "ERROR: DeepSeek-V4 MTP on MI325X currently supports GPU-resident KV only" >&2
    exit 1
fi

Every other single-node none-only agentic vLLM recipe in the repo (dsv4_fp4_b300_vllm_mtp.sh, dsv4_fp4_b200_vllm_mtp.sh, kimik3_fp4_b300_vllm_mtp.sh, kimik3_fp4_b300_vllm.sh) instead calls require_agentic_kv_offload_none, defined in benchmark_lib.sh:27-36.

Why this is more than cosmetic

require_agentic_kv_offload_none does two things, not one:

require_agentic_kv_offload_none() {
    if agentic_kv_offload_enabled; then
        echo "Error: expected KV_OFFLOADING=none, got '$KV_OFFLOADING'" >&2
        exit 1
    fi
    if [[ -n "${KV_OFFLOAD_BACKEND:-}" ]]; then
        echo "Error: KV_OFFLOAD_BACKEND must be empty when KV_OFFLOADING=none" >&2
        exit 1
    fi
}
  1. Rejects KV_OFFLOADING != none (the half this script reimplements).
  2. Rejects a nonempty KV_OFFLOAD_BACKEND even when KV_OFFLOADING=none — catching a misconfiguration where a backend name is left set despite offloading being disabled.

The new script only reimplements check (1) and silently drops check (2).

Code path that triggers it / why nothing else catches it

check_env_vars (line 9 of the new script) only validates that required vars are set, not their cross-field consistency — it has no knowledge of the KV_OFFLOAD_BACKEND/KV_OFFLOADING relationship. So if some future launcher, CI matrix expansion, or manual override left KV_OFFLOAD_BACKEND set to a stale value (e.g., mooncake or hicache) alongside KV_OFFLOADING=none, this script would proceed silently — the value is simply never read since KV offloading is disabled — whereas the four sibling scripts would fail fast with a clear error at startup.

Step-by-step proof

  1. Suppose an operator's environment (or a copy-pasted launch script) sets export KV_OFFLOAD_BACKEND=mooncake alongside export KV_OFFLOADING=none for this recipe — e.g. left over from testing a DRAM-offload variant.
  2. In dsv4_fp8_mi325x_mtp.sh, line 13's check only inspects $KV_OFFLOADING, which is "none", so the check passes and the script continues to launch vLLM normally.
  3. In any sibling recipe (e.g. dsv4_fp4_b300_vllm_mtp.sh), the same environment would call require_agentic_kv_offload_none, which additionally checks KV_OFFLOAD_BACKEND is empty, and exits immediately with Error: KV_OFFLOAD_BACKEND must be empty when KV_OFFLOADING=none.
  4. The inconsistency means this recipe is silently more permissive than its siblings for the exact same misconfiguration class, defeating the purpose of having a shared, fail-fast validation helper.

Fix

Replace lines 13-16 with:

require_agentic_kv_offload_none

(optionally keeping the DeepSeek-V4/MI325X-specific context in a preceding comment, since the shared helper's error message is generic).

This is a quality/consistency finding — the omitted check is currently harmless for the config actually being added, since every search-space entry in configs/amd-master.yaml for this recipe sets kv-offloading: none with no backend. It should be fixed for consistency with the rest of the codebase, but does not block this PR from merging.


source "$(dirname "$0")/../../benchmark_lib.sh"

check_env_vars MODEL IMAGE TP CONC KV_OFFLOADING RESULT_DIR DURATION EP_SIZE DP_ATTENTION

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.

🟡 check_env_vars on line 11 requires IMAGE, but the script body never references $IMAGE anywhere — it appears to be a leftover copied from the sibling minimaxm3_fp8_mi325x.sh, which genuinely uses IMAGE (line 160) to branch on nightly-tag behavior. Since this recipe pins a stable v0.26.0 image with no such branching, IMAGE should simply be dropped from the check_env_vars list.

Extended reasoning...

What the bug is: benchmarks/single_node/agentic/dsv4_fp8_mi325x_mtp.sh line 11 declares IMAGE as a required environment variable via check_env_vars MODEL IMAGE TP CONC KV_OFFLOADING RESULT_DIR DURATION EP_SIZE DP_ATTENTION, but nothing in the rest of the 150-line script ever reads $IMAGE. A grep -n IMAGE over the file returns exactly one hit — the check_env_vars line itself.

Where it likely came from: The sibling script benchmarks/single_node/agentic/minimaxm3_fp8_mi325x.sh requires the exact same variable list shape (MODEL IMAGE TP CONC KV_OFFLOADING ... RESULT_DIR DURATION EP_SIZE DP_ATTENTION) and does use $IMAGE meaningfully at line 160: if [[ \"$IMAGE\" == vllm/vllm-openai-rocm:nightly-* ]]; then, which gates nightly-image-specific behavior (that script runs against a floating nightly tag). This new dsv4 script pins a stable, versioned image (vllm/vllm-openai-rocm:v0.26.0 in configs/amd-master.yaml) and has no equivalent conditional, strongly suggesting the check_env_vars line was copied wholesale from the minimaxm3 script without dropping the now-unused IMAGE entry.

Why nothing else catches this: check_env_vars is a generic helper (in benchmark_lib.sh) that only asserts the named variables are non-empty in the caller's environment — it has no way to know whether the script actually consumes each one, so a dead entry like this passes silently. Every other agentic recipe without image-gated branching (dsv4_fp4_b200/b300/mi355x, glm5.2, minimaxm3 on h100/h200/mi300x, etc.) simply omits IMAGE from its check_env_vars call, confirming that requiring it here is not a repo-wide convention but an artifact of the copy.

Impact: This is low severity — the harness that invokes this script always supplies IMAGE from the config's image: field (as seen in configs/amd-master.yaml's dsv4-fp8-mi325x-vllm-agentic-mtp entry), so check_env_vars never actually fails in practice. The real cost is to readability and future maintenance: a reader sees IMAGE in the required-vars list and reasonably expects some image-based branching later in the script (as the minimaxm3 sibling has), then finds none — this can lead to confusion or, worse, someone adding dead conditional logic to 'complete' a feature that was never actually needed here.

Proof (step-by-step):

  1. Run grep -n IMAGE benchmarks/single_node/agentic/dsv4_fp8_mi325x_mtp.sh → only line 11 (check_env_vars ...) matches; zero uses in the script body.
  2. Run the same grep on minimaxm3_fp8_mi325x.sh → two matches: the check_env_vars declaration (line 7) AND a live consumer at line 160 (if [[ \"$IMAGE\" == vllm/vllm-openai-rocm:nightly-* ]]).
  3. Compare against every other agentic script that doesn't branch on image tag (e.g. dsv4_fp4_mi355x_vllm_agentic_mtp.sh, glm5.2 scripts, minimaxm3 h100/h200/mi300x variants) — none of them list IMAGE in check_env_vars at all.
  4. Conclusion: IMAGE in this script's check_env_vars is dead weight copied from the minimaxm3 sibling, not a repo convention.

How to fix: Simply drop IMAGE from the check_env_vars call on line 11, i.e. change it to check_env_vars MODEL TP CONC KV_OFFLOADING RESULT_DIR DURATION EP_SIZE DP_ATTENTION. If image-gated behavior is actually intended for a future nightly build, add the conditional logic instead and keep the requirement.

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@cquil11 cquil11 added full-sweep-enabled and removed full-sweep-enabled agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable labels Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@cquil11 cquil11 added agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable sweep-enabled and removed full-sweep-enabled agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable labels Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@cquil11
cquil11 force-pushed the agent/dsv4-mi325x-agentic-mtp branch from 3c84765 to 7d02823 Compare August 10, 2026 17:37
@cquil11 cquil11 added the agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@cquil11 cquil11 removed the agentx-fast Run AgentX throughput with 1 warmup request per lane and a 20-minute profile; not reusable label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

2 similar comments
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

/stage-results 31460097931

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@cquil11 staged run 31460097931: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-11~r31460097931

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

@cquil11 已将运行 31460097931 发布到预发布环境:https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-11~r31460097931

后续的 /stage-results 请求不会移除此运行;再次发布相同的运行 ID 会更新其预发布数据。预发布工作流

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.

1 participant