feat(amd): orchestrate AMD clusters with srt-slurm - #2542
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 关于重新运行失败任务的文档 |
Document the MI300X aggregate and disaggregated validation plan, cluster assumptions, safety boundaries, and current srt-slurm development pin.
cquil11
changed the title
3d6a75a to
0641575
Compare
Add the reusable MI300X cluster runtime profile and a minimal stable-vLLM fixed-sequence aggregate recipe for functional bring-up.
cquil11
changed the title
|
Claude finished @cquil11's task in 8m 2s —— View job PR Review — feat(amd): orchestrate AMD clusters with srt-slurm
SummaryNo blocking issues found. Two 🟡 warnings posted inline:
Checks that passed: Note: I could not execute |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| --container-image="$SHARED_IMAGE" \ | ||
| --container-mounts="$SHARED_HF_CACHE:/hf_hub_cache" \ | ||
| --container-writable --container-remap-root --no-container-entrypoint \ | ||
| --export=ALL,HF_HOME=/hf_hub_cache,HF_HUB_CACHE=/hf_hub_cache,HUGGINGFACE_HUB_CACHE=/hf_hub_cache,MODEL_REPO=${MODEL} \ |
There was a problem hiding this comment.
🟡 WARNING: The staging job downloads the model with HF_HUB_CACHE=/hf_hub_cache (flat root), but the Qwen3.5 production recipe was deliberately aligned to /hf_hub_cache/hub in commit d3ed345 (disagg-1p1d-tp8-fixed-seq.yaml sets HF_HUB_CACHE: "/hf_hub_cache/hub" for workers and the benchmark).
Why it matters: For the qwen3.5-fp8-mi355x-sglang-srt-disagg lane, this staging download lands in /it-share/hf-hub-cache/models--Qwen--... while the workers resolve /it-share/hf-hub-cache/hub/models--.... On a cold cache the stage job spends its 45-minute limit fetching ~400 GB into a root the recipe never reads, and the serve path still has to fetch the model again under hub/. Your validation run passed because hub/ was already warm — a fresh cluster or cache eviction breaks this lane, and the flat-root copy is duplicated dead weight on shared storage.
Fix: Drop the explicit overrides so snapshot_download uses the default $HF_HOME/hub, matching the production recipe:
| --export=ALL,HF_HOME=/hf_hub_cache,HF_HUB_CACHE=/hf_hub_cache,HUGGINGFACE_HUB_CACHE=/hf_hub_cache,MODEL_REPO=${MODEL} \ | |
| --export=ALL,HF_HOME=/hf_hub_cache,MODEL_REPO=${MODEL} \ |
Note this leaves the two qwen3-0.6b recipes (which read the flat root) re-fetching a 0.6B model at serve time — trivial, but ideally their HF_HUB_CACHE/HUGGINGFACE_HUB_CACHE should also move to /hf_hub_cache/hub so all MI355X srt lanes share one cache layout.
| test -r /raid/hf-hub-cache/inferencex/srt-slurm/containers/vllm-openai-rocm-v0.26.0.sqsh | ||
| test -r /raid/hf-hub-cache/inferencex/srt-slurm/containers/vllm-router-nightly-20260809-d2ba586.sqsh |
There was a problem hiding this comment.
🟡 WARNING: This launch script uses container images but never transforms a public Docker image into the enroot squashfs — it only test -rs two pre-existing node-local .sqsh files and fails if they're absent. The MI355X launcher in this same PR has the right pattern (enroot import -o "$tmp" "docker://${SGLANG_IMAGE}" fallback under a lock); the MI300X stage job already runs on every eligible node, so it's the natural place to do the same.
Why it matters: Reproducibility. Nobody can re-run this lane on a fresh or re-imaged node without out-of-band manual imports, and AMD_BRINGUP.md documents the node-local RAID constraint but not how these files were produced. In particular, vllm-router-nightly-20260809-d2ba586.sqsh has no public image reference anywhere in the PR (the vLLM engine at least maps to vllm/vllm-openai-rocm:v0.26.0), so that artifact is currently unreproducible from this repo.
Fix: In the stage srun block, import from the public registry when the file is missing, e.g.:
img_dir=/raid/hf-hub-cache/inferencex/srt-slurm/containers
[[ -r "$img_dir/vllm-openai-rocm-v0.26.0.sqsh" ]] || \
enroot import -o "$img_dir/vllm-openai-rocm-v0.26.0.sqsh" docker://vllm/vllm-openai-rocm:v0.26.0and either do the same for the router image or document (here or in AMD_BRINGUP.md) the exact public source/build steps for vllm-router-nightly-20260809-d2ba586.sqsh.
There was a problem hiding this comment.
Automated review ran; one nit-level bug was found and posted inline (MI355X qwen3.5 staging writing to the wrong HF cache root). Beyond that, a few other candidates were checked and ruled out: the producer-SHA file written by the srt-slurm launchers, the MI355X sglang disagg conc-list vs. the custom benchmark's actual concurrency sweep, and the AMD_BRINGUP.md translation/pin-staleness questions — none of these turned out to be functional issues. Given the size and novelty of this orchestration integration (new launcher scripts, cluster profiles, and recipes), a human look is still worthwhile.
Extended reasoning...
This PR introduces a large, brand-new AMD orchestration surface (srt-slurm integration): two new launcher scripts with nontrivial Slurm/Pyxis staging logic, two cluster profiles, five new recipe YAMLs, and two new contract test files, spanning MI300X and MI355X. Given the scale, the novelty of the code paths (remote staging via sbatch/srun, cache-path plumbing, git-pinned runtime fetch), and that CI already flags this PR for CODEOWNER sign-off, this falls well outside the bar for full automated approval regardless of the bug severity found. The one inline finding is a nit (redundant download risk, not a hard failure per the PR's own successful validation run), so it does not itself block merging, but the overall change warrants a human pass.
| srun --nodes=1 --ntasks=1 \ | ||
| --container-image="$SHARED_IMAGE" \ | ||
| --container-mounts="$SHARED_HF_CACHE:/hf_hub_cache" \ | ||
| --container-writable --container-remap-root --no-container-entrypoint \ | ||
| --export=ALL,HF_HOME=/hf_hub_cache,HF_HUB_CACHE=/hf_hub_cache,HUGGINGFACE_HUB_CACHE=/hf_hub_cache,MODEL_REPO=${MODEL} \ | ||
| python3 -c 'import os; from huggingface_hub import snapshot_download; snapshot_download(os.environ["MODEL_REPO"])' |
There was a problem hiding this comment.
🟡 The MI355X staging step's snapshot_download always writes to the flat /hf_hub_cache root, but the qwen3.5 production recipe (disagg-1p1d-tp8-fixed-seq.yaml) sets the serving env's HF_HUB_CACHE/HUGGINGFACE_HUB_CACHE to /hf_hub_cache/hub — so the prefetched ~400GB checkpoint lands one directory above where the SGLang backend looks, causing a cache miss and a redundant re-download at serve time. Fix by aligning the staging srun's cache env (runners/launch_mi355x-amds-srt.sh:62-67) to /hf_hub_cache/hub for this lane, or reverting the recipe to the flat path.
Extended reasoning...
What happens: huggingface_hub's snapshot_download resolves its cache directory directly from the HF_HUB_CACHE env var when it is set (it only falls back to $HF_HOME/hub when neither HF_HUB_CACHE nor HUGGINGFACE_HUB_CACHE are present). The staging srun in runners/launch_mi355x-amds-srt.sh:62-67 explicitly sets HF_HOME=HF_HUB_CACHE=HUGGINGFACE_HUB_CACHE=/hf_hub_cache before calling snapshot_download($MODEL), so the pre-fetched model snapshot is written directly under /hf_hub_cache/models--Qwen--Qwen3.5.../.
Where it breaks: The production recipe benchmarks/multi_node/srt-slurm-recipes/sglang/qwen3.5/mi355x/disagg-1p1d-tp8-fixed-seq.yaml sets the prefill/decode/benchmark env for that same model (Qwen/Qwen3.5-397B-A17B-FP8) to HF_HUB_CACHE=HUGGINGFACE_HUB_CACHE=/hf_hub_cache/hub (lines 53-58 and 143-145) — one directory below where staging wrote it. When the SGLang worker starts, it resolves its cache to /hf_hub_cache/hub/models--Qwen--Qwen3.5.../, finds nothing there, and re-downloads the ~400GB FP8 checkpoint over the network instead of reusing the prefetched snapshot.
Why this slipped through: Commit d3ed345 ("fix(amd): align Qwen3.5 Hugging Face cache root") deliberately moved the recipe's env vars to the /hub suffix, with an in-file comment explaining the goal was to keep "srt-slurm's prefetch and every backend process" aligned on the identical snapshot path. That commit updated the recipe but never touched this launcher's staging step, which still hardcodes the flat root — an incomplete fix. The smaller qwen3-0.6b agg/disagg recipes are unaffected because they still use the flat /hf_hub_cache path, which happens to match staging.
Step-by-step proof:
- Staging srun runs with
HF_HUB_CACHE=/hf_hub_cache, callssnapshot_download("Qwen/Qwen3.5-397B-A17B-FP8"). huggingface_hubwrites the snapshot to/hf_hub_cache/models--Qwen--Qwen3.5-397B-A17B-FP8/snapshots/<rev>/....srtctl applylaunches the prefill/decode SGLang workers withHF_HUB_CACHE=/hf_hub_cache/hub(per the recipe).- SGLang's model loader resolves the cache to
/hf_hub_cache/hub/models--Qwen--Qwen3.5-397B-A17B-FP8/..., which does not exist. - It falls through to a full network download of the ~400GB FP8 checkpoint into that path, defeating the purpose of the dedicated staging allocation.
Impact: wasted staging work, doubled disk usage in the shared HF cache, and a re-download inside the serving job's health-check window (720 attempts × 5s = 60 min), which is a real reliability risk if the compute nodes have constrained outbound network access, as the accompanying bring-up doc implies they might.
Why nit rather than normal: the PR's own validation (Slurm job 38267 on head 3b3acff) completed green for exactly this qwen3.5 production lane, so the redundant download did not hard-fail in practice — most likely because the shared /it-share/hf-hub-cache was already warm from the pre-existing qwen3.5-fp8-mi355x-sglang lane (which uses the same model at a different path), or srt-slurm's own internal prefetch (aligned to $HF_HOME/hub) independently populated the correct path. The concrete effect today is wasted prefetch work and doubled disk usage rather than a guaranteed failure, though a cold cache would trigger the described re-download risk.
Fix: change the staging srun's exported HF_HUB_CACHE/HUGGINGFACE_HUB_CACHE to /hf_hub_cache/hub for this lane (matching the recipe), or drop the /hub suffix from the recipe to match the flat staging path used by the other lanes.
…ntegration # Conflicts: # perf-changelog.yaml
1 participant
Summary
Integrates the SemiAnalysisAI srt-slurm AMD/ROCm work into InferenceX without Dynamo.
utils/bench_serving/benchmark_serving.pythrough srt-slurm's custom benchmarkScope boundary
Dynamo, NATS, etcd, NIXL, and Dynamo's NVIDIA-specific frontend/runtime assumptions are intentionally excluded from the AMD paths. MI300X uses direct vLLM or native vLLM Router; MI355X uses native SGLang Router. KV transfer uses MoRIIO for vLLM and AMD MoRI for SGLang.
Validation
The final review-head runs are on exact InferenceX head
e22bf36a306acc18545ea1475efa0493d063f278. MI300X disaggregate Slurm job11787completed0:0on two nodes, and MI300X aggregate Slurm job11790completed0:0on one node. MI355X production Slurm job38274completed0:0on two nodes. Native SGLang Router reported exactly one healthy prefill and one healthy decode worker. Runtime identity checks passed for SGLang0.5.17.dev20260809+g7120f3ee13, SGLang Router0.3.2, and AMD MoRI at the matching SGLang commit.The unchanged MI355X fixed-sequence benchmark completed 8/8 warmups and 16/16 measured ISL 8192 / OSL 1024 / c8 requests with no HTTP, router, MoRI registration, HIP/OOM, or NCCL failure. It reached 0.83 req/s, 847.73 output tok/s, and 7,629.56 total tok/s. The final review fix aligns MI355X staging and every serving role on
$HF_HOME/hub; the realized runtime prefetch and both worker environments confirmed/hf_hub_cache/hub. MI300X now recreates missing node-local engine and router squashfs files atomically from the pinned public images. Both final MI300X runs passed c1 and c4 fixed-sequence requests, and all result JSON, runtime logs, aggregation, and success-rate jobs uploaded successfully.Static validation
perf-changelog.yamlfinal newline and generated matrix validation against currentorigin/mainCompanion PR