Skip to content

port MI355X DeepSeek V4 ATOM disagg to srt-slurm - #2627

Open
cquil11 wants to merge 9 commits into
agent/srt-slurm-amd-integrationfrom
agent/port-dsv4-mi355x-atom-srt-slurm
Open

port MI355X DeepSeek V4 ATOM disagg to srt-slurm#2627
cquil11 wants to merge 9 commits into
agent/srt-slurm-amd-integrationfrom
agent/port-dsv4-mi355x-atom-srt-slurm

Conversation

@cquil11

@cquil11 cquil11 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Scope and dependencies

Port only the active MI355X DeepSeek-V4-Pro FP4 ATOM 8k/1k disaggregated configuration to native srt-slurm ATOM workers and AToMesh. No Infera, Dynamo, or performance tuning.

Shared InferenceX integration: #2542 (0d7ca6f95). Runtime: srt-slurm #7, stacked on #1, pinned at 81d46274f508e18ab14d1f123b75132005818dcf. This PR includes latest main d13df3789. Relative to #2542, the diff is four files: two recipes, the master entry, and the append-only changelog.

Review base: #2542. The complete native PR sweep ran while temporarily targeting main, as required by run-sweep.yml; the validated commit is unchanged. No workflow gate was bypassed.

Preserved workload and explicit engine settings

  • 1P1D TP8: c4, 8, 16, 32, 64, 128; two 8-GPU nodes.
  • 2P1D TP8 with DP attention/TBO: c256, 512, 768, 1024, 2048; three 8-GPU nodes.
  • Mooncake default RDMA; FP8 KV cache; block size 16; GPU memory utilization 0.85; context 9472; prefill admission 256; decode admission 128/2048; original decode graph sizes and prefix-cache disablement.
  • Original router policy/flags, input/output lengths, random-length ratio 0.8, warmup/request counts, GSM8K workload and 91% threshold.
  • Unchanged benchmark_serving.py, invoked through the custom benchmark interface.

A structural comparison against the previously tested head confirms both recipes retain identical backend options, topology, frontend options, benchmark commands, and Slurm limits.

Explicit image change — official upstream only

The image changes from rocm/atom-dev:nightly_202606101403 to the unmodified official rocm/atom-dev:nightly_202609031453 (registry digest sha256:f4dc16b802d3e5326cbbb73055b621fb0d6264e0f982ed2389d91182a976fd7f). Native ATOM/AToMesh revision: f95ef3ec30a16876a676e0af96d60f8e0605c7c5.

The old image ignored max_completion_tokens after AToMesh normalized max_tokens. GSM8K requested 5376 output tokens, but the old ATOM API used its 8192-token default; a 2P1D decode worker then exceeded its 9472-token context capacity. The API fix is already upstream in ROCm/ATOM #1318 and included in this nightly.

This is an engine-image refresh, not merely an orchestration change, and requires fresh performance and accuracy validation. There are no source patches, cherry-picks, overlays, custom images, or setup hooks. The obsolete Python 3.10 Mooncake library override is removed so the Python 3.12 image uses its own library paths.

Clean ownership

The existing launch_mi355x-amds.sh is the single entry point. srt-slurm owns allocation, workers/router, readiness, logging, and completion; Pyxis owns containers. The shared adapter maps CI inputs and collects artifacts. The extra srt launcher, host-repair helper, RDMA preflight helper, GPU-drain loops, and separate staging allocation are removed in #2542. No shared-host repair is performed by these recipes.

Validation status

  • Previous run 33782782708, attempt 2: all 11 throughput points completed every requested benchmark request; 1P1D GSM8K passed at 96.5883% over 1319 examples. The 2P1D eval crashed as described above. The run is failed, not eligible for staging.
  • Latest local checks: 270 InferenceX matrix/changelog tests; 147 srt-slurm ATOM/frontend/accelerator/Slurm/dry-run tests; 14 token-limit cases against unmodified upstream ATOM protocol. Both recipes dry-run successfully. Changelog validates against both main and feat(amd): orchestrate AMD clusters with srt-slurm #2542.
  • Native full PR run 33825304569 is completely green, attempt 1, on head fe8e9f9bf: all 11 throughput points and 48,600/48,600 measured requests; both complete GSM8K evaluations (1,319 examples each); result aggregation and changelog metadata checks.
  • Accuracy: 1P1D strict 96.73995% / flexible 96.66414%; 2P1D strict 96.36088% / flexible 96.43669%. The unchanged threshold is 91%. Per-sample audits confirmed every response is nonempty and reproduced both scores. Canonical benchmark aggregation contains exactly the expected 6 + 5 concurrency points and correct runtime identities.
  • Six 2P1D eval requests waited for native KV completion notifications and recovered on the unchanged client's first automatic retry after its 1,800-second timeout. No engine patch, config workaround, manual job cancellation, injected request, or workflow rerun was used. The original notification stall's root cause is not established; this is not presented as a bug fixed by the port.
  • Validation covers orchestration, throughput/latency, and full accuracy. Power telemetry was not collected (power_valid=0); the optional srt-slurm dashboard is not the source of the custom benchmark results.
  • Results are staged and available for review. The staging workflow completed successfully, including ingestion, chart refresh, and the PR callback.

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

@cquil11

cquil11 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Live 2P/1D DPA validation update:\n\n- Slurm 39443 completed all 64 model shards and CUDA-graph capture on both prefills and the decode.\n- Startup then failed deterministically in every logical worker because each ATOM data-parallel EngineCore tried to bind the same KV-event ZMQ endpoint. One DP rank bound successfully; the others raised Address already in use from Infera BlockManager hook initialization, so the worker could not register.\n- This is an Infera multi-DP KV-event publisher bug, not Slurm, MI355X hardware, model loading, Mooncake, or the srt-slurm topology.\n- The upstream fix is open as AMD-AGI/Infera#121. It gives every DP rank a local PUB ingress and relays all rank streams through the one endpoint advertised for the logical worker, retaining complete KV-aware cache visibility.\n- Focused engine/router validation is green: 50 tests passed, including two simultaneous EngineCore publishers through one advertised endpoint; Ruff, pre-commit, and diff checks passed.\n- The current exact-head workflow was not cancelled. Its 1P/1D arm is still running naturally as Slurm 39448. I will pin the reviewed fix and launch a fresh exact-head validation only after this attempt closes.

@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch from 8f0a94a to 0eb21ea Compare August 27, 2026 00:23
@cquil11
cquil11 changed the base branch from agent/atom-infera-validation to main August 27, 2026 00:23
@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch 6 times, most recently from 61b4789 to 5e21bed Compare August 27, 2026 16:24
@cquil11
cquil11 changed the base branch from main to agent/srt-slurm-amd-integration August 27, 2026 16:25
@cquil11
cquil11 force-pushed the agent/srt-slurm-amd-integration branch from 8581966 to 1d4522c Compare August 27, 2026 16:28
@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch from 5e21bed to 4c4a61b Compare August 27, 2026 16:28
@cquil11
cquil11 force-pushed the agent/srt-slurm-amd-integration branch from 1d4522c to 6da0605 Compare August 27, 2026 17:27
@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch from 4c4a61b to 514a519 Compare August 27, 2026 17:27
@cquil11
cquil11 marked this pull request as ready for review August 27, 2026 17:30
@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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


Review complete

  • Gather context (diff vs agent/srt-slurm-amd-integration, read changed files)
  • Review the two new srt-slurm recipe YAMLs
  • Review configs/amd-master.yaml changes (image accessibility, config key format)
  • Validate perf-changelog.yaml entry placement and content
  • Post review feedback

Summary

Found 1 blocking issue and 1 minor issue (posted as inline comments):

  1. 🔴 srt-slurm pin discrepancy (perf-changelog.yaml:6571): the changelog claims PR7 is pinned at 10a58d2e…, but the effective pin is SRT_SLURM_COMMIT=c87d7b34… in runners/launch_mi355x-amds-srt.sh:8, which this PR does not touch (and the PR body cites a third SHA, 76e7d76…, as the validated head). The new recipes require PR7's backend: atom / frontend: atomesh / mooncake_protocol support, so if the launcher pin predates PR7, both topology jobs fail at srtctl apply. Bump the launcher pin to the validated PR7 head, or correct the changelog if c87d7b34 really includes ATOM support.
  2. 🟡 Stray comment (configs/amd-master.yaml:913): dangling duplicated # 1P1D TP8 copy-paste leftover at end of the new block.

Everything else checked out: the changelog entry is correctly appended at the end of the file and both master-config and changelog were updated together; dsv4 is an established config-key prefix in this repo; rocm/atom-dev:nightly_202608251555 is a public Docker Hub image; the conc-list: [1] + recipe-owned concurrency loop follows the parent PR's established srt-slurm pattern; the result contract (/results/${SLURM_JOB_ID}/fixed-seq/*-c<N>.json) matches the launcher's collection/parsing logic; all benchmark_serving.py flags used (--backend openai, --num-warmups, --percentile-metrics, --trust-remote-code) exist; and the launcher's DP-attn mutation hook (which only knows sglang_config) is safely skipped for these entries since ep: 1.

Note: the inferencemax-repos MCP server failed to connect this session, and a cross-repo gh api lookup of the srt-slurm SHAs required approval, so I could not externally confirm what c87d7b34 contains — the pin finding is based on the in-repo discrepancy.

Comment thread perf-changelog.yaml Outdated
- "Use the hardware-proven Mooncake TCP transport for the initial correctness sweep; validate the updated ROCm RDMA path separately before changing production transport."
- "Use the ATOM image's bundled native AToMesh binary directly, without Infera source overlays or Infera routing, and archive the complete runtime logs from the custom benchmark."
- "Run the current rocm/atom-dev nightly_202608251555 image at ATOM commit 4e0848dcdf94f3d4be0c1e3ce51db99bc93f8b8c, whose bundled native AToMesh and Mooncake implementation are used without downstream source patches."
- "Pin SemiAnalysisAI/srt-slurm PR #7 at 10a58d2ebb4d756c423424049c736b909b72e14f so this ATOM/AToMesh port shares the latest validated AMD orchestration base, including native-router data-parallel rank preservation and ATOM recipe flag normalization."

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.

🔴 BLOCKING: This changelog entry claims srt-slurm PR #7 is pinned at 10a58d2ebb4d756c423424049c736b909b72e14f, but nothing in this PR (or the repo) actually pins that commit. The shared launcher runners/launch_mi355x-amds-srt.sh:8 still has SRT_SLURM_COMMIT="c87d7b34b009be920896126013ad6dc74c5a99d5", which was set by the parent PR (#2542) for the SGLang-native lanes, and this PR does not modify the launcher.

Why it matters: The two new recipes depend on srt-slurm PR7 features (backend: type: atom, frontend: type: atomesh, connector: mooncake, mooncake_protocol, atom_config). If c87d7b34 predates PR7's native ATOM/AToMesh support, srtctl apply will fail schema validation for both topology jobs and the sweep can never pass. Note the PR body also cites a third SHA (76e7d76961…) as the validated PR7 head, so the intended pin is ambiguous — right now three different commits are referenced and only c87d7b34 is effective.

Fix: Bump SRT_SLURM_COMMIT in runners/launch_mi355x-amds-srt.sh to the exact PR7 head you validated against (and make the changelog text match that SHA), or if c87d7b34 genuinely contains the ATOM/AToMesh support, correct this changelog line to the real pin. Fix this →

@cquil11

cquil11 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Replacement native full-sweep validation: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33717234735

Head df1481b, shared base #2542 at 0f390f6, srt-slurm PR #7 pinned to 527525d43ce2c1d33aa1d429aae0d8f338637b49. Synced with upstream main 6829ed5; the ATOM recipe YAML and AMD matrix are byte-for-byte unchanged from the previous attempt.

This corrects an orchestrator defect: native ATOMesh was mistakenly sent through Dynamo NATS/etcd startup and failed before any engine worker started. No engine/image/transport/workload change. The new regression test covers ATOMesh, SGLang, and Dynamo infrastructure selection. Existing validated #2555 and #2630 heads remain unchanged.

Expected validation: 1P1D c4,8,16,32,64,128; 2P1D c256,512,768,1024,2048; both native GSM8K evaluations. Results will only be staged after the full native run and its artifacts are checked.

Comment thread configs/amd-master.yaml Outdated
@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch from df1481b to d3b8c38 Compare September 3, 2026 05:44
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Current native full sweep: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33720270808

Head d3b8c38; shared base #2542 at 01676c3; runtime PR #7 at 527525d43ce2c1d33aa1d429aae0d8f338637b49. The engine recipes and AMD matrix are unchanged from the preceding attempt.

The previous run got past NATS/etcd startup but hit the workflow-to-container Python cache-prefix leak. Both 1P1D jobs failed before weight loading; the two still-queued 2P1D jobs were canceled, without touching unrelated jobs. The exact-image CPU-only A/B probe (Slurm 42144, 1 CPU, zero GPUs) reproduced the import failure with the inherited prefix and passed with it absent. The fix is in the shared launcher, not the model settings.

This new run was triggered by full-sweep-enabled after the new head passed its standalone changelog gate. It must validate all 11 throughput points and both native accuracy evaluations before results are staged.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cquil11
cquil11 force-pushed the agent/port-dsv4-mi355x-atom-srt-slurm branch from d3b8c38 to 1fb90fe Compare September 3, 2026 17:07
@cquil11

cquil11 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Resuming ATOM only. Fresh full-sweep-enabled validation: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33782782708 on 1fb90fe. Runtime fa994733 fixes the confirmed evaluation model-name mismatch; shared base 38be99b restores legacy unlimited RDMA memlock. No engine options, images, transport, concurrency lists, benchmark script, or accuracy thresholds changed. Prior run 33720270808 produced five successful 2P1D throughput points, but 1P1D throughput and both evals failed, so nothing from it is staged. The GPU/NIC registration probe 42195 passed all eight devices; memlock A/B probe 42198 is queued. 420 focused local runtime tests pass. Need all 11 throughput points and both evaluations green before staging. MiniMax M3 FP4 vLLM remains next, after ATOM completes.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Validation update for run 33782782708, head 1fb90fe / runtime fa994733:

  • 1P1D throughput passed all six concurrencies (4, 8, 16, 32, 64, 128). The original benchmark JSON confirms completed == num_prompts for every point: 40, 80, 160, 320, 640, 1280.
  • 1P1D GSM8K eval passed: 96.5883% on 1,319 examples, versus the unchanged 91% DSV4 threshold.
  • Both 2P1D jobs failed before engine startup with fatal: reference is not a tree: fa994733.... My runtime rebase made the old pin absent from fresh clones before those queued jobs began. This is a publication error, not an ATOM model/runtime failure.

I restored the exact old commit under immutable-reference tag inferencex-runtime-fa9947333212, also retained the new cleanup pin as inferencex-runtime-81d46274f508, and verified a fresh clone checks out fa994733 successfully. Reran only failed jobs with gh run rerun 33782782708 --failed; successful 1P1D jobs remain preserved.

https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33782782708

No engine/workload config or scheduler bin-packing policy was changed. The later single-entrypoint cleanup still requires its own full native PR sweep after this baseline run.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Fresh native full PR sweep on the unmodified official ATOM nightly and clean single-entrypoint launcher: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33825304569 (head fe8e9f9, runtime 81d46274f508e18ab14d1f123b75132005818dcf). No patches, custom images, overlays, or setup hooks. Includes all 11 throughput points and both complete GSM8K evals. The previous run is failed because its old native ATOM API dropped the requested token limit; its successful throughput and 1P1D accuracy results are preserved, not being reused as a substitute for this fresh full run.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fe8e9f9. Configure here.

echo "[clear_caches] ${base_url}: L3 clear http=${code:-000} (optional backend unavailable)"
fi
done
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache flush hits wrong worker APIs

Medium Severity

The new flush posts SGLang /flush_cache to every AIPERF_SERVER_METRICS_URLS target, a list already used for vLLM/TRT drain scraping. Failed flushes retry for FLUSH_DRAIN_TIMEOUT per worker, so non-SGLang agentic jobs stall, and SGLang recipes that never set that list skip the reset and reuse prefix cache across concurrencies.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fe8e9f9. Configure here.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@cquil11

cquil11 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

/stage-results 33825304569

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@cquil11 staged run 33825304569: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-09-04~r33825304569

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

@cquil11
cquil11 changed the base branch from main to agent/srt-slurm-amd-integration September 4, 2026 04:12
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