[AMD][AgentX] Port MI355X DeepSeek V4 MTP disagg to srt-slurm - #2555
[AMD][AgentX] Port MI355X DeepSeek V4 MTP disagg to srt-slurm#2555cquil11 wants to merge 21 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 关于重新运行失败任务的文档 |
8d65951 to
2c09431
Compare
…x-agentx-srt-slurm
* perf(agentx): retune DSV4 B300 MTP sweep Update the vLLM image and concurrency grid, raise the DEP4 host-memory budget, and set long-prefill-token-threshold to 512 for DEP4 and DEP8. 中文：更新 vLLM 镜像与并发配置，提高 DEP4 主机内存预算，并为 DEP4 和 DEP8 将 long-prefill-token-threshold 设为 512。 * docs(changelog): link DSV4 B300 MTP PR Replace the temporary changelog placeholder with draft PR #2536. 中文：将变更日志中的临时占位符替换为草稿 PR #2536 的链接。 * perf(agentx): extend DEP8 SimpleCPU sweep Move every DEP8 point to vLLM SimpleCPU offload, widen the concurrency grid through c512, and drop DEP4 c40. 中文：将全部 DEP8 配置切换到 vLLM SimpleCPU 卸载，将并发扫描扩展至 c512，并移除 DEP4 c40。 * perf(agentx): finalize SimpleCPU sweep grid Move TP4 c16 to SimpleCPU offload, skip TP4 c12 and DEP8 c64, and extend the DEP8 sweep through c576. 中文：将 TP4 c16 切换到 SimpleCPU 卸载，移除 TP4 c12 与 DEP8 c64，并将 DEP8 扫描扩展至 c576。 * Update perf-changelog.yaml --------- Co-authored-by: Cameron Quilici <cjquilici@gmail.com>
…x-agentx-srt-slurm # Conflicts: # perf-changelog.yaml
|
Claude finished @cquil11's task in 12m 0s —— View job PR Review
SummaryFound 2 blocking issues and 1 warning (posted as inline comments):
Checked and clean: enroot |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - config-keys: | ||
| - dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp | ||
| description: | ||
| - "Port the merged MI355X DeepSeek V4 AgentX MTP 1P1D search space from the legacy AMD multinode launcher to srt-slurm." | ||
| - "Use srt-slurm's native SGLang Router frontend and AMD MoRI transport without Dynamo, NATS, etcd, or NIXL." | ||
| - "Preserve TP8/EP1 c1,c2,c4,c8,c16,c32 and TP8/EP8/DPA c96,c128, EAGLE MTP, the golden simulated average length, HiCache settings, and logical prefill/decode server-metrics collection through the custom AgentX benchmark." | ||
| - "Remove the superseded DSV4-specific amd_utils benchmark wrapper, model stanza, HiCache override plumbing, and hard-coded synthetic-acceptance injection; the srt-slurm recipes now own those settings directly." | ||
| - "Allocate all 128 logical CPUs per MI355X backend node so SGLang TP8 affinity follows the complete host topology, and pin srt-slurm diagnostics that preserve failure-log tails containing non-UTF-8 ROCm compiler output." | ||
| - "Move the known-green DeepSeek V4 ROCm kernel-routing contract into each self-contained srt-slurm recipe; this avoids the unsupported AITer fused-MoE heuristic dispatch path while keeping legacy harness-only environment indirection removed." | ||
| - "Declare the exact SGLang and AMD MoRI runtime fingerprint reported by the pinned ROCm image so srt-slurm identity verification is reproducible." | ||
| - "Rebase the AMD runtime on the active native-router branch and wait for every advertised SGLang HTTP worker to become healthy before launching Model Gateway, preventing slow ROCm startup from permanently dropping a static prefill worker after the router's one-shot registration timeout." | ||
| - "Pin the MI355X launcher to SemiAnalysisAI/srt-slurm commit 5c0d5a718194198447e8ceea5c37048c2fb8eaf5, including the native-router base and SGLang worker-readiness gate." | ||
| - "Document the MI355X Ionic DSCP/PFC and RDMA health contract required by MoRI, including the drain-before-repair and verify-before-resume procedure." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2555 | ||
|
|
||
| - config-keys: | ||
| - dsv4-fp4-b300-vllm-agentic-mtp | ||
| description: | ||
| - "Update the B300 DSV4 AgentX MTP image and SimpleCPU sweep through c576." |
There was a problem hiding this comment.
🔴 BLOCKING: New perf-changelog.yaml entries must be appended to the END of the file. The file is read chronologically (oldest at top, newest at bottom), so inserting in the middle or prepending breaks the ordering. The #2536 entry (merged to main via the branch update) now sits after this PR's #2555 entry — please move the #2555 entry to the bottom of the file.
Fix:
| - config-keys: | |
| - dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp | |
| description: | |
| - "Port the merged MI355X DeepSeek V4 AgentX MTP 1P1D search space from the legacy AMD multinode launcher to srt-slurm." | |
| - "Use srt-slurm's native SGLang Router frontend and AMD MoRI transport without Dynamo, NATS, etcd, or NIXL." | |
| - "Preserve TP8/EP1 c1,c2,c4,c8,c16,c32 and TP8/EP8/DPA c96,c128, EAGLE MTP, the golden simulated average length, HiCache settings, and logical prefill/decode server-metrics collection through the custom AgentX benchmark." | |
| - "Remove the superseded DSV4-specific amd_utils benchmark wrapper, model stanza, HiCache override plumbing, and hard-coded synthetic-acceptance injection; the srt-slurm recipes now own those settings directly." | |
| - "Allocate all 128 logical CPUs per MI355X backend node so SGLang TP8 affinity follows the complete host topology, and pin srt-slurm diagnostics that preserve failure-log tails containing non-UTF-8 ROCm compiler output." | |
| - "Move the known-green DeepSeek V4 ROCm kernel-routing contract into each self-contained srt-slurm recipe; this avoids the unsupported AITer fused-MoE heuristic dispatch path while keeping legacy harness-only environment indirection removed." | |
| - "Declare the exact SGLang and AMD MoRI runtime fingerprint reported by the pinned ROCm image so srt-slurm identity verification is reproducible." | |
| - "Rebase the AMD runtime on the active native-router branch and wait for every advertised SGLang HTTP worker to become healthy before launching Model Gateway, preventing slow ROCm startup from permanently dropping a static prefill worker after the router's one-shot registration timeout." | |
| - "Pin the MI355X launcher to SemiAnalysisAI/srt-slurm commit 5c0d5a718194198447e8ceea5c37048c2fb8eaf5, including the native-router base and SGLang worker-readiness gate." | |
| - "Document the MI355X Ionic DSCP/PFC and RDMA health contract required by MoRI, including the drain-before-repair and verify-before-resume procedure." | |
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2555 | |
| - config-keys: | |
| - dsv4-fp4-b300-vllm-agentic-mtp | |
| description: | |
| - "Update the B300 DSV4 AgentX MTP image and SimpleCPU sweep through c576." | |
| - config-keys: | |
| - dsv4-fp4-b300-vllm-agentic-mtp | |
| description: | |
| - "Update the B300 DSV4 AgentX MTP image and SimpleCPU sweep through c576." | |
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2536 | |
| - config-keys: | |
| - dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp | |
| description: | |
| - "Port the merged MI355X DeepSeek V4 AgentX MTP 1P1D search space from the legacy AMD multinode launcher to srt-slurm." | |
| - "Use srt-slurm's native SGLang Router frontend and AMD MoRI transport without Dynamo, NATS, etcd, or NIXL." | |
| - "Preserve TP8/EP1 c1,c2,c4,c8,c16,c32 and TP8/EP8/DPA c96,c128, EAGLE MTP, the golden simulated average length, HiCache settings, and logical prefill/decode server-metrics collection through the custom AgentX benchmark." | |
| - "Remove the superseded DSV4-specific amd_utils benchmark wrapper, model stanza, HiCache override plumbing, and hard-coded synthetic-acceptance injection; the srt-slurm recipes now own those settings directly." | |
| - "Allocate all 128 logical CPUs per MI355X backend node so SGLang TP8 affinity follows the complete host topology, and pin srt-slurm diagnostics that preserve failure-log tails containing non-UTF-8 ROCm compiler output." | |
| - "Move the known-green DeepSeek V4 ROCm kernel-routing contract into each self-contained srt-slurm recipe; this avoids the unsupported AITer fused-MoE heuristic dispatch path while keeping legacy harness-only environment indirection removed." | |
| - "Declare the exact SGLang and AMD MoRI runtime fingerprint reported by the pinned ROCm image so srt-slurm identity verification is reproducible." | |
| - "Rebase the AMD runtime on the active native-router branch and wait for every advertised SGLang HTTP worker to become healthy before launching Model Gateway, preventing slow ROCm startup from permanently dropping a static prefill worker after the router's one-shot registration timeout." | |
| - "Document the MI355X Ionic DSCP/PFC and RDMA health contract required by MoRI, including the drain-before-repair and verify-before-resume procedure." | |
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2555 |
| amd-mori: "0.5.15.post1.dev20260719+ga03ca46a28" | ||
|
|
||
| slurm: | ||
| time_limit: "04:00:00" |
There was a problem hiding this comment.
🔴 BLOCKING: This row runs six concurrency points (conc-list: [1,2,4,8,16,32]) inside one srt-slurm job, but the standard sweep passes DURATION=3600 per point (DEFAULT_AGENTIC_DURATION_SECONDS in utils/matrix_logic/validation.py:16; the 1200s fast path only applies with the agentx-fast label). build_replay_cmd passes that straight to --benchmark-duration, so profiling alone is ≥6h — before DSV4 model load, per-point dataset configuration (AIPERF_DATASET_CONFIGURATION_TIMEOUT=1800), per-point warmup grace (up to 1800s), and inter-point drain waits.
Why it matters: time_limit: "04:00:00" guarantees the Slurm job hits its wall and is killed mid-sweep on any non-fast run (main-branch sweeps, full-sweep-enabled PR runs). The legacy path this ports from ran under #SBATCH --time=24:00:00 (benchmarks/multi_node/amd_utils/job.slurm:8).
Fix:
| time_limit: "04:00:00" | |
| time_limit: "24:00:00" |
| benchmark: | ||
| type: custom | ||
| command: | | ||
| set -euo pipefail | ||
| mkdir -p /infmax-workspace/LOGS/agentic | ||
| export RESULT_DIR=/infmax-workspace/LOGS/agentic | ||
| export AGENTIC_OUTPUT_DIR=/infmax-workspace | ||
| exec bash /infmax-workspace/benchmarks/multi_node/agentic_srt.sh |
There was a problem hiding this comment.
🟡 WARNING: This is the first lane to run agentic_srt.sh against the native SGLang Router + SGLang backends, and its inter-point drain gate (wait_for_agentic_servers_idle, benchmarks/multi_node/agentic_srt.sh:32-98) only knows Dynamo/vLLM metric names: it polls the frontend for dynamo_frontend_active_requests and the workers for vllm:num_requests_running / vllm:num_requests_waiting.
Why it matters: with this stack neither name exists, so between concurrency points one of two things happens:
- If
GET /metricson the router port returns 200, all sums are 0 and the gate trivially passes after 3 polls — no actual drain, so in-flight requests from the previous point bleed into the next point's measurements. - If the router does not serve
/metricson the request port (sglang-router exposes Prometheus on a separate port), every poll raises, and after 1800s the script exits non-zero with "Agentic servers did not drain" — killing the run after the first point. Withconc-list: [1,2,4,8,16,32]this path executes 5 times (the c96/c128 rows are single-point and unaffected).
Fix: teach the drain check the SGLang names (sglang:num_running_reqs on the worker metrics endpoints srt-slurm injects via AIPERF_SERVER_METRICS_URLS, and skip or adapt the frontend query for FRAMEWORK=sglang-disagg), or at minimum confirm the in-progress live validation covers a multi-point transition on this row before undraft. Fix this →
| forwarded = ( | ||
| "AIPERF_EXPERIMENTAL_FAST", | ||
| "CONC", | ||
| "CONC_LIST", | ||
| "DURATION", | ||
| "EVAL_CONC", | ||
| "EVAL_LIMIT", | ||
| "EVAL_ONLY", | ||
| "FRAMEWORK", | ||
| "IS_AGENTIC", | ||
| "KV_OFFLOADING", | ||
| "MAX_MODEL_LEN", | ||
| "MODEL", | ||
| "MODEL_PREFIX", | ||
| "PRECISION", | ||
| "RESULT_FILENAME", | ||
| "RUN_EVAL", | ||
| "RUNNER_TYPE", | ||
| "SPEC_DECODING", | ||
| "TOTAL_CPU_DRAM_GB", | ||
| ) |
There was a problem hiding this comment.
🔴 runners/launch_mi355x-amds-srt.sh:100-121 defines the srt-slurm forwarding list that copies the launcher's env into the recipe's benchmark.env, but it omits PREFILL_NUM_WORKERS, PREFILL_TP, PREFILL_EP, PREFILL_DP_ATTN, DECODE_NUM_WORKERS, DECODE_TP, DECODE_EP, DECODE_DP_ATTN, DISAGG, and IMAGE. Since the new dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp recipes hardcode IS_MULTINODE: "true", process_agentic_result.py's _gpu_shape() reads these via env_int()/env_bool() inside the container and gets all-default (0/false/empty) values, so every concurrency point of this new lane will publish num_prefill_gpu=0, num_decode_gpu=0, tp=0, disagg=false, image='', and silently drop the per_gpu throughput block.
Extended reasoning...
The bug. runners/launch_mi355x-amds-srt.sh builds the srt-slurm recipe's benchmark.env from a curated forwarded tuple (the launcher's own env is not otherwise visible inside the srt-slurm benchmark container — this curated list is the only channel):
forwarded = (
"AIPERF_EXPERIMENTAL_FAST", "CONC", "CONC_LIST", "DURATION", "EVAL_CONC",
"EVAL_LIMIT", "EVAL_ONLY", "FRAMEWORK", "IS_AGENTIC", "KV_OFFLOADING",
"MAX_MODEL_LEN", "MODEL", "MODEL_PREFIX", "PRECISION", "RESULT_FILENAME",
"RUN_EVAL", "RUNNER_TYPE", "SPEC_DECODING", "TOTAL_CPU_DRAM_GB",
)It never forwards PREFILL_NUM_WORKERS, PREFILL_TP, PREFILL_EP, PREFILL_DP_ATTN, DECODE_NUM_WORKERS, DECODE_TP, DECODE_EP, DECODE_DP_ATTN, DISAGG, or IMAGE — even though all of them are present in the launcher's own environment (IMAGE is even required at line 18 via : "${IMAGE:?...}", and DISAGG/PREFILL_NUM_WORKERS/PREFILL_TP/DECODE_NUM_WORKERS/DECODE_TP are read later in the same script at lines 207-212 to compute TOTAL_GPUS).
The code path that triggers it. The three new recipe YAMLs added in this PR (disagg-1p1d-tp8-mtp-hicache.yaml, disagg-1p1d-dep8-mtp-hicache-c96.yaml, disagg-1p1d-dep8-mtp-hicache-c128.yaml) all hardcode IS_MULTINODE: "true" in their benchmark.env block, and their benchmark.command execs agentic_srt.sh, which runs the aiperf trace replay and then calls write_agentic_result_json -> python3 -m utils.agentic.aggregation.process_agentic_result inside the srt-slurm container. process_agentic_result.py::_gpu_shape() checks is_multinode = env_bool("IS_MULTINODE") (line 99); since it's true, it takes the multinode branch (lines 116-150) and reads PREFILL_NUM_WORKERS/PREFILL_TP/PREFILL_EP and DECODE_NUM_WORKERS/DECODE_TP/DECODE_EP via env_int() (defaults to 0 when unset), PREFILL_DP_ATTN/DECODE_DP_ATTN via env_bool() (defaults to False), and image = os.environ.get("IMAGE", "") and disagg = env_bool("DISAGG") (both empty/false by default) in build_agg().
Why nothing catches it. These vars do exist in the launcher's shell environment (configs/amd-master.yaml's search-space entries set PREFILL_TP, PREFILL_NUM_WORKERS, DECODE_TP, etc. as job-level env, and the workflow sets DISAGG/IMAGE), so a naive read of the launcher script would suggest the topology is known. But the container running process_agentic_result.py only sees whatever was written into the recipe's benchmark.env by this curated forwarded list — the srt-slurm container does not inherit the launcher's ambient shell environment. The workflow's success check (benchmark-multinode-tmpl.yml, globbing result files and checking num_requests_successful) only verifies that files exist, not that their topology fields are correct, so the run "passes" while quietly emitting corrupted metadata.
Impact. For every concurrency point of the new dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp config (c1/c2/c4/c8/c16/c32 TP8, plus c96/c128 DEP8), the agg_*.json will record num_prefill_gpu=0, num_decode_gpu=0, num_gpus=0, tp=prefill_tp+decode_tp=0, ep=1 (fallback), disagg=false, image=''. Because num_gpus=0, the num_gpus > 0 guard in build_agg() (right before agg["request_metrics"] = request_nested) also silently skips computing request_nested["throughput"]["per_gpu"], dropping tput_per_gpu/output_tput_per_gpu/input_tput_per_gpu — the primary per-GPU throughput metrics this benchmark exists to produce. The run does not crash (every division here is guarded), so it reports success with garbage/incomplete data for the entire new lane, requiring a full re-sweep once discovered. Notably, the PR's own job.slurm diff documents fixing this exact class of bug for the legacy AMD launcher path (missing PREFILL_TP/DECODE_TP/DISAGG forwarding causing tp=0/prefill_tp=0/disagg=false); it is reintroduced here in the new srt-slurm Python forwarding list.
Step-by-step proof. Take the c1 arm of dsv4-fp4-mi355x-sglang-disagg-agentic-hicache-mtp (TP8/EP1, non-DP): (1) the launcher's shell env has PREFILL_TP=8, PREFILL_NUM_WORKERS=1, DECODE_TP=8, DECODE_NUM_WORKERS=1, DISAGG=true, IMAGE=lmsysorg/sglang-rocm:.... (2) The forwarded tuple copies only MODEL, CONC, RUNNER_TYPE, etc. into the recipe's benchmark.env; none of the six topology vars or DISAGG/IMAGE are copied. (3) srt-slurm launches the benchmark container using only that benchmark.env. (4) Inside the container, agentic_srt.sh runs the trace replay then calls process_agentic_result.py. (5) _gpu_shape() calls env_int("PREFILL_NUM_WORKERS") -> 0 (unset), env_int("PREFILL_TP") -> 0, likewise for decode -> num_prefill_gpu = 0*0 = 0, num_decode_gpu = 0, num_gpus = 0, tp = 0+0 = 0. (6) build_agg() sets disagg = env_bool("DISAGG") -> False, image = os.environ.get("IMAGE", "") -> "". (7) Because num_gpus == 0, the if "total_tput_tps" in request_flat and num_gpus > 0: branch is skipped, so request_nested["throughput"] never gets a per_gpu key. (8) The resulting agg_*.json for conc1 (and every other concurrency point) has num_prefill_gpu: 0, num_decode_gpu: 0, tp: 0, disagg: false, image: "" and no tput_per_gpu/output_tput_per_gpu/input_tput_per_gpu, even though the run itself served real traffic across 16 real GPUs.
Fix. Add PREFILL_NUM_WORKERS, PREFILL_TP, PREFILL_EP, PREFILL_DP_ATTN, DECODE_NUM_WORKERS, DECODE_TP, DECODE_EP, DECODE_DP_ATTN, DISAGG, and IMAGE to the forwarded tuple in runners/launch_mi355x-amds-srt.sh so they land in the recipe's benchmark.env the same way the other process_agentic_result.py inputs (MODEL, RUNNER_TYPE, etc.) already do.
| mkdir -p "${SRT_REPO_DIR}/$(dirname "$CONFIG_PATH")" | ||
| cp "$LOCAL_RECIPE" "${SRT_REPO_DIR}/${CONFIG_PATH}" | ||
| cp "$CLUSTER_PROFILE" "${WORK_DIR}/srtslurm.yaml" | ||
| python3 - "${WORK_DIR}/srtslurm.yaml" "$GITHUB_WORKSPACE" "$SHARED_RESULTS" <<'PY' | ||
| python3 - "${WORK_DIR}/srtslurm.yaml" "${SRT_REPO_DIR}/${CONFIG_PATH}" \ | ||
| "$GITHUB_WORKSPACE" "$SHARED_RESULTS" "$SHARED_AIPERF_CACHE" "$SHARED_IMAGE" <<'PY' | ||
| import os | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| path = Path(sys.argv[1]) | ||
| workspace, results = sys.argv[2:] | ||
| import yaml | ||
|
|
||
| profile_path = Path(sys.argv[1]) | ||
| recipe_path = Path(sys.argv[2]) | ||
| workspace, results, aiperf_cache, image_path = sys.argv[3:] | ||
| needle = " /it-share/hf-hub-cache: /hf_hub_cache\n" | ||
| text = path.read_text() | ||
| text = profile_path.read_text() | ||
| if text.count(needle) != 1: | ||
| raise SystemExit("expected exactly one Hugging Face cache mount") | ||
| path.write_text( | ||
| profile_path.write_text( | ||
| text.replace( | ||
| needle, | ||
| needle + f" {workspace}: /infmax-workspace\n {results}: /results\n", | ||
| needle | ||
| + f" {aiperf_cache}: /aiperf_mmap_cache\n" | ||
| + f" {workspace}: /infmax-workspace\n" | ||
| + f" {results}: /results\n", | ||
| ) | ||
| ) | ||
|
|
||
| recipe = yaml.safe_load(recipe_path.read_text()) | ||
| container_alias = recipe["model"]["container"] |
There was a problem hiding this comment.
🟡 This PR deletes utils/test_mi300x_srt_slurm_contract.py outright instead of just updating the one stale SHA-pin assertion it contained, removing all CI coverage for the still-active runners/launch_mi300x-amds-srt.sh launcher and its MI300X recipes/config entries. It also leaves the substantial new logic in runners/launch_mi355x-amds-srt.sh added by this PR (IMAGE_KEY/SHARED_IMAGE derivation, the recipe-patching Python block, forwarded-env injection, EVAL_ONLY/RUN_EVAL mutation) with no contract test at all.
Extended reasoning...
This PR removes utils/test_mi300x_srt_slurm_contract.py in its entirety (225 lines, commit ab17f87 "drop redundant launcher contract suite"). That file was the only test covering runners/launch_mi300x-amds-srt.sh — it asserted the SRT_SLURM_COMMIT pin, --no-preflight submission mode, VLLM_IMAGE/VLLM_ROUTER_IMAGE values, the enroot/flock atomic-staging sequence, and the exact agg/disagg search-space entries in configs/amd-master.yaml. None of that is duplicated anywhere else in the tree (a repo-wide grep for the launcher, the vllm/qwen3-0.6b/mi300x recipes, and SRTCTL_RUNTIME_SOURCE_DIR turns up nothing else). All of that MI300X code is still present, still active, and entirely untouched by this PR's MI355X-porting scope.
One verifier refutation makes a fair point worth addressing directly: the deleted test was stale — it hard-asserted the old MI300X pin 315e4b06a7e0806194a646ea21832e750e896a46, and this PR's own AMD_BRINGUP.md diff bumps that pin to dd0109d4..., so the assertion would have failed as-is and the author had to do something about it. That's true, but it only justifies fixing one line. The other assertions in the file (--no-preflight, the image tags, the enroot/flock staging sequence, the recipe/config contract) were still accurate at deletion time and covered code this PR doesn't touch. Choosing to delete the whole 225-line suite rather than update the single stale line trades a real, working regression test for no test at all — on a launcher that continues to run in CI.
Compounding this, the substantial new logic this PR adds to runners/launch_mi355x-amds-srt.sh:87-116 (the IMAGE→IMAGE_KEY/SHARED_IMAGE derivation, the inline-Python block that patches the recipe's container alias, forwarded-env, and EVAL_ONLY/RUN_EVAL mutation) ships with zero test coverage of its own — no test_mi355x_srt_slurm_contract.py or equivalent exists anywhere in the tree after this diff.
Concretely, a future regression in launch_mi300x-amds-srt.sh (e.g. an accidental change to --no-preflight, the pinned images, or the enroot staging locking) would previously have been caught by test_mi300x_srt_slurm_contract.py::test_official_matrix_routes_disagg_through_the_pinned_srt_launcher and its siblings; after this PR, nothing in CI would catch it.
This is a coverage-regression concern, not a runtime correctness bug — the code itself still works, so nothing breaks by merging as-is. The straightforward fix is to restore the deleted test with just the one stale pin assertion updated (315e4b06... → dd0109d4...), and separately add a new test_mi355x_srt_slurm_contract.py covering the recipe-patching logic this PR introduces.
| SGLANG_DISAGGREGATION_WAITING_TIMEOUT: "1800" | ||
| decode_environment: | ||
| <<: *common_environment | ||
| SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK: "1024" |
There was a problem hiding this comment.
🟡 In the legacy amd_utils launcher, env.sh unconditionally pins SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE=128 for DeepSeek-V4-Pro, and server_sglang.sh's ${MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE:-...} expansion always preferred that pinned 128 over the DEP8/conc/MTP-derived fallback — so every validated legacy DSV4 AgentX arm actually ran decode at dispatch-tokens-per-rank=128. These three new srt-slurm recipes instead hardcode the derived formula value that was actually shadowed: disagg-1p1d-tp8-mtp-hicache.yaml:97 sets 1024, disagg-1p1d-dep8-mtp-hicache-c96.yaml:97 sets 48, and disagg-1p1d-dep8-mtp-hicache-c128.yaml:97 sets 64 — none matches the validated 128. srt-slurm never sources env.sh, so nothing restores it; this silently changes decode MoE dispatch-buffer sizing on every arm despite the PR description's claim to preserve the validated configuration.
Extended reasoning...
The bug: the legacy launcher's actually-validated decode MoRI dispatch-tokens-per-rank value is 128 for DeepSeek-V4-Pro AgentX, but none of the three new srt-slurm recipes reproduce it — they instead reproduce the derived value that the legacy launcher silently overrode and never actually used.
Code path: benchmarks/multi_node/amd_utils/env.sh (line ~341, in the if [[ \"$MODEL_NAME\" == \"DeepSeek-V4-Pro\" ]] block) unconditionally exports MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE=128, with a comment noting server_sglang.sh prefers these over the MORI_MAX_DISPATCH_* coupling when set. Sure enough, server_sglang.sh's decode launch builds SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK=${MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE:-${MORI_MAX_DISPATCH_TOKENS_DECODE}}. Because the PER_RANK var is always non-empty for this model, the :- fallback to the DEP8/conc/MTP-derived MORI_MAX_DISPATCH_TOKENS_DECODE (the value the DP+EP override block in server_sglang.sh actually computes from concurrency/topology) never fires. Nothing in server_sglang.sh ever reassigns the PER_RANK var. So on the legacy path, every validated DSV4 AgentX arm — TP8, DEP8 c96, DEP8 c128 — launched decode with per-rank dispatch tokens pinned at 128, regardless of what the derived formula would have produced.
Why the PR doesn't prevent it: srt-slurm recipes don't source env.sh at all — they set the decode container's environment directly via decode_environment.SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK in the YAML. The author baked in the derived formula values (which they likely computed from the same conc/DEP8/MTP formula server_sglang.sh uses) not realizing that on the legacy path this derived value was always shadowed by the fixed 128 override. As a result:
disagg-1p1d-tp8-mtp-hicache.yaml:97→\"1024\"disagg-1p1d-dep8-mtp-hicache-c96.yaml:97→\"48\"disagg-1p1d-dep8-mtp-hicache-c128.yaml:97→\"64\"
None of these is 128. Since srt-slurm never sources env.sh, there is no later override to restore the validated value — the recipe's YAML is authoritative.
Corroborating evidence: the same recipes correctly preserve the prefill pin — line 92 in all three sets SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK: \"16384\", exactly matching env.sh's MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_PREFILL=16384. This prefill/decode asymmetry is strong evidence the decode value was re-derived from the formula rather than copied from the validated pinned value, contradicting the PR description's explicit claim to 'preserve the existing validated configuration.'
Step-by-step proof (TP8 arm):
- Legacy launch for DSV4-Pro AgentX TP8 sets
MODEL_NAME=DeepSeek-V4-Pro, which triggersenv.sh's block, exportingMORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE=128. server_sglang.shbuildsDECODE_CMDwithSGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK=${MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE:-...}→ since the var is set to 128 (non-empty), bash's:-never evaluates the fallback → the sglang decode server actually launches withSGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK=128.- In the new srt-slurm recipe
disagg-1p1d-tp8-mtp-hicache.yaml,decode_environment.SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANKis set directly to\"1024\"in the YAML. - srt-slurm passes this environment straight into the decode container; there is no
env.shsourcing step to override it. - Result: the decode server launches with dispatch-tokens-per-rank=1024 instead of the validated 128 — an 8x change to MoE dispatch buffer sizing versus what was actually benchmarked in the referenced PR [AMD][AgentX] DSv4 MI355X agentic disagg: add EAGLE/MTP speculative-decoding arm / DSv4 MI355X 智能体分离式推理：新增 EAGLE/MTP 投机解码分支 #2309.
Impact: this is a benchmark-fidelity divergence, not a crash — all three hardcoded values (1024, 48, 64) are usable per-rank caps for their respective topologies (48/64 match the tight DEP8 per-step token bound; 1024 over-provisions TP8), so the runs will complete and produce results, just under different MoE dispatch sizing than what was actually validated.
Fix: hardcode 128 (matching env.sh's pinned MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK_DECODE) for decode_environment.SGLANG_MORI_NUM_MAX_DISPATCH_TOKENS_PER_RANK in all three recipes, the same way the prefill side already correctly hardcodes 16384.
* [AMD] [WIP] [AGENTX] Adding GLM5.2 MI355X Support - Agentx Signed-off-by: Giovanni Guasti <giovanni.guasti@amd.com> * [AMD] [WIP] [AGENTX] Adding GLM5.2 MI355X Support - Agentx Signed-off-by: Giovanni Guasti <giovanni.guasti@amd.com> * [AMD] [WIP] [AGENTX] Adding GLM5.2 MI355X Support - Agentx Signed-off-by: Giovanni Guasti <giovanni.guasti@amd.com> * chore: update pr-link for glm5.2-fp4-mi355x-sglang-agentic-mtp 中文：更新 glm5.2-fp4-mi355x-sglang-agentic-mtp 配置的 pr-link 至 PR #2488 * [AMD] [WIP] [AGENTX] GLM 5.2 - Update glm5.2_fp4_mi355x_sglang_mtp.sh * [AMD] [WIP] [AGENTX] GLM 5.2 - Update perf-changelog.yaml * [AMD] [WIP] [AGENTX] GLM 5.2 - Update glm5.2_fp4_mi355x_sglang_mtp.sh * fix(glm5.2-fp4-mi355x-sglang-mtp): remove trailing backslashes from SGLANG_CMD array Backslashes inside bash arrays are treated as literal characters, causing SGLang to receive arguments like 'EAGLE \' instead of 'EAGLE', which results in 'unrecognized arguments' errors. 中文：修复 SGLANG_CMD 数组中多余的行继续符（反斜杠），避免 SGLang 将其作为字面参数解析导致启动失败 * fix(glm5.2-fp4-mi355x-sglang-mtp): increase watchdog-timeout to 3600s MTP draft-token forward passes under long-context agentic load block the SGLang scheduler long enough to trip the 1800s watchdog, killing the server mid-warmup. Matches the value used by multi-node AMD disagg recipes. 中文：将 watchdog-timeout 从 1800s 增加至 3600s，避免 MTP 投机解码在长上下文推理负载下触发 SGLang 调度器看门狗超时导致服务崩溃 * [AMD] [WIP] [AGENTX] GLM 5.2 - Update glm5.2_fp4_mi355x_sglang_mtp.sh * fix(glm5.2-fp4-mi355x-sglang-agentic-mtp): set mem-fraction-static 0.80 for conc<=16 / 修正低并发场景 mem-fraction-static 参数为 0.80 0.85 OOMs the device mid-replay on 131072-token prefill chunks (observed: "Tried to allocate 6.86 GiB ... 5.15 GiB is free", run 29751563205). The comment already documented 0.80 as the correct value; the assignment was wrong. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [AMD][AgentX] glm5.2 fp4 mi355x sglang mtp: set SGLANG_SIMULATE_ACC_LEN=2.99 for non-eval runs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AMD] [AGENTX] GLM 5.2 - Update glm5.2_fp4_mi355x_sglang_mtp.sh * [AMD] [AGENTX] GLM 5.2 - Update amd-master.yaml * [AMD] [AGENTX] GLM 5.2 Update Search Space --------- Signed-off-by: Giovanni Guasti <giovanni.guasti@amd.com> Co-authored-by: ajith-sirra-amd <122240613+ajith-sirra-amd@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: seungrokj <144636725+seungrokj@users.noreply.github.com> Co-authored-by: seungrokj <seungrok.jung@amd.com>
…x-agentx-srt-slurm # Conflicts: # perf-changelog.yaml
4 participants
Summary
Validation
Relationship
This is intentionally stacked on #2542. The srt-slurm runtime changes live in SemiAnalysisAI/srt-slurm#1, rebased on the native router work in SemiAnalysisAI/srt-slurm#2.