feat: validate ATOM and Infera on MI300X and MI355X - #2558
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 关于重新运行失败任务的文档 |
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
* 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>
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
…idation # Conflicts: # perf-changelog.yaml
cquil11
changed the title
|
Claude finished @cquil11's task in 8m 2s —— View job PR Review
SummarySolid validation PR — pinned commits, exact identity checks, real hardware evidence, and a green final-head run. I left 2 🟡 inline comments, both the same class of bug: the new Infera/ATOM git source staging is not serialized, while the container import right next to it deliberately flocks for the identical concurrent agg+disagg scenario. On MI355X ( Checks that passed review:
Two non-blocking notes: (1) I could not execute the pytest suites or |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if [[ ! -d "${SHARED_INFERA_RUNTIME}/.git" ]]; then | ||
| git clone --quiet "${INFERA_REPOSITORY}" "${SHARED_INFERA_RUNTIME}" | ||
| fi | ||
| git -C "${SHARED_INFERA_RUNTIME}" fetch --quiet origin "${INFERA_COMMIT}" | ||
| git -C "${SHARED_INFERA_RUNTIME}" checkout --quiet --detach "${INFERA_COMMIT}" | ||
| test "\$(git -C "${SHARED_INFERA_RUNTIME}" rev-parse HEAD)" = "${INFERA_COMMIT}" | ||
| if [[ ! -d "${SHARED_ATOM_RUNTIME}/.git" ]]; then | ||
| git clone --quiet "${ATOM_REPOSITORY}" "${SHARED_ATOM_RUNTIME}" | ||
| fi | ||
| git -C "${SHARED_ATOM_RUNTIME}" fetch --quiet origin "${ATOM_COMMIT}" | ||
| git -C "${SHARED_ATOM_RUNTIME}" checkout --quiet --detach "${ATOM_COMMIT}" | ||
| test "\$(git -C "${SHARED_ATOM_RUNTIME}" rev-parse HEAD)" = "${ATOM_COMMIT}" |
There was a problem hiding this comment.
🟡 WARNING: The Infera/ATOM git staging on the shared filesystem runs before the flock is taken (the lock at line 84 only covers the squashfs import), so the header comment "The shared lock makes concurrent aggregate/disaggregated validations safe" doesn't hold for these clones.
Why it matters: The agg and disagg matrix rows are separate CI jobs that can stage concurrently. On first bring-up of a new pin, two simultaneous git clones into the same /it-share/.../runtime/... directory make the loser fail (destination path already exists and is not an empty directory), and even with an existing checkout, concurrent git fetch/checkout --detach on the same worktree can collide on .git/index.lock. With set -euo pipefail that kills the stage job — a flaky, hard-to-attribute CI failure. Note the per-image ${SHARED_IMAGE}.lock wouldn't serialize this even if reordered, since an sglang row and an atom row use different lock files but stage the same source trees.
Fix: Serialize the source staging with its own shared lock:
| if [[ ! -d "${SHARED_INFERA_RUNTIME}/.git" ]]; then | |
| git clone --quiet "${INFERA_REPOSITORY}" "${SHARED_INFERA_RUNTIME}" | |
| fi | |
| git -C "${SHARED_INFERA_RUNTIME}" fetch --quiet origin "${INFERA_COMMIT}" | |
| git -C "${SHARED_INFERA_RUNTIME}" checkout --quiet --detach "${INFERA_COMMIT}" | |
| test "\$(git -C "${SHARED_INFERA_RUNTIME}" rev-parse HEAD)" = "${INFERA_COMMIT}" | |
| if [[ ! -d "${SHARED_ATOM_RUNTIME}/.git" ]]; then | |
| git clone --quiet "${ATOM_REPOSITORY}" "${SHARED_ATOM_RUNTIME}" | |
| fi | |
| git -C "${SHARED_ATOM_RUNTIME}" fetch --quiet origin "${ATOM_COMMIT}" | |
| git -C "${SHARED_ATOM_RUNTIME}" checkout --quiet --detach "${ATOM_COMMIT}" | |
| test "\$(git -C "${SHARED_ATOM_RUNTIME}" rev-parse HEAD)" = "${ATOM_COMMIT}" | |
| mkdir -p "${SHARED_BASE}/runtime" | |
| exec 8>"${SHARED_BASE}/runtime/.source-staging.lock" | |
| flock -w 2400 8 | |
| if [[ ! -d "${SHARED_INFERA_RUNTIME}/.git" ]]; then | |
| git clone --quiet "${INFERA_REPOSITORY}" "${SHARED_INFERA_RUNTIME}" | |
| fi | |
| git -C "${SHARED_INFERA_RUNTIME}" fetch --quiet origin "${INFERA_COMMIT}" | |
| git -C "${SHARED_INFERA_RUNTIME}" checkout --quiet --detach "${INFERA_COMMIT}" | |
| test "\$(git -C "${SHARED_INFERA_RUNTIME}" rev-parse HEAD)" = "${INFERA_COMMIT}" | |
| if [[ ! -d "${SHARED_ATOM_RUNTIME}/.git" ]]; then | |
| git clone --quiet "${ATOM_REPOSITORY}" "${SHARED_ATOM_RUNTIME}" | |
| fi | |
| git -C "${SHARED_ATOM_RUNTIME}" fetch --quiet origin "${ATOM_COMMIT}" | |
| git -C "${SHARED_ATOM_RUNTIME}" checkout --quiet --detach "${ATOM_COMMIT}" | |
| test "\$(git -C "${SHARED_ATOM_RUNTIME}" rev-parse HEAD)" = "${ATOM_COMMIT}" | |
| flock -u 8 |
| ensure_git_checkout() { | ||
| local target="\$1" | ||
| local repository="\$2" | ||
| local commit="\$3" | ||
| local temporary="\${target}.tmp.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| local quarantine="\${target}.incomplete.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| if [[ ! -d "\$target/.git" ]]; then | ||
| if [[ -e "\$target" ]]; then | ||
| mv "\$target" "\$quarantine" | ||
| fi | ||
| git clone --quiet "\$repository" "\$temporary" | ||
| git -C "\$temporary" fetch --quiet origin "\$commit" | ||
| git -C "\$temporary" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$temporary" rev-parse HEAD)" = "\$commit" | ||
| mv "\$temporary" "\$target" | ||
| else | ||
| git -C "\$target" fetch --quiet origin "\$commit" | ||
| git -C "\$target" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$target" rev-parse HEAD)" = "\$commit" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🟡 WARNING: ensure_git_checkout has no locking, unlike ensure_container_image directly above it, which flocks ${target}.lock to guard exactly this concurrent-stage-jobs scenario (agg + disagg rows staging on the same node at the same time, since /raid paths are keyed only by commit, not by run).
Why it matters: If two stage jobs race on first bring-up of a new pin, both see no $target/.git, both clone to their unique temporaries, and both run mv "$temporary" "$target". The second mv doesn't fail — because $target now exists as a directory, it silently nests the second clone inside it (e.g. runtime/atom-<sha>/atom-<sha>.tmp.<job>.<pid>/), leaving a stray full clone inside the runtime source tree that gets mounted into workers as /atom-source. In the already-exists path, concurrent git fetch/checkout on the same worktree can also collide on .git/index.lock and kill the job under set -euo pipefail.
Fix: Reuse the existing lock pattern — take flock on "\${target}.lock" around the body of ensure_git_checkout (re-checking [[ -d "\$target/.git" ]] after acquiring), and/or use mv -T "\$temporary" "\$target" so a lost race fails loudly instead of silently nesting.
| if [[ ! -d "${SHARED_INFERA_RUNTIME}/.git" ]]; then | ||
| git clone --quiet "${INFERA_REPOSITORY}" "${SHARED_INFERA_RUNTIME}" | ||
| fi | ||
| git -C "${SHARED_INFERA_RUNTIME}" fetch --quiet origin "${INFERA_COMMIT}" | ||
| git -C "${SHARED_INFERA_RUNTIME}" checkout --quiet --detach "${INFERA_COMMIT}" | ||
| test "\$(git -C "${SHARED_INFERA_RUNTIME}" rev-parse HEAD)" = "${INFERA_COMMIT}" | ||
| if [[ ! -d "${SHARED_ATOM_RUNTIME}/.git" ]]; then | ||
| git clone --quiet "${ATOM_REPOSITORY}" "${SHARED_ATOM_RUNTIME}" | ||
| fi | ||
| git -C "${SHARED_ATOM_RUNTIME}" fetch --quiet origin "${ATOM_COMMIT}" | ||
| git -C "${SHARED_ATOM_RUNTIME}" checkout --quiet --detach "${ATOM_COMMIT}" | ||
| test "\$(git -C "${SHARED_ATOM_RUNTIME}" rev-parse HEAD)" = "${ATOM_COMMIT}" | ||
| exec 9>"${SHARED_IMAGE}.lock" | ||
| flock -w 2400 9 |
There was a problem hiding this comment.
🔴 The new Infera/ATOM git clone/fetch/checkout --detach steps against the commit-keyed SHARED_INFERA_RUNTIME/SHARED_ATOM_RUNTIME dirs (and the analogous REMOTE_INFERA_RUNTIME/REMOTE_ATOM_RUNTIME in the MI300X launcher) have no locking, unlike the sibling container-image staging just below which is explicitly wrapped in flock (whose own comment says the lock is what makes concurrent agg/disagg validations safe).
Extended reasoning...
The bug. In runners/launch_mi355x-amds-srt.sh the stage sbatch script clones/fetches/checks-out INFERA_COMMIT/ATOM_COMMIT into SHARED_INFERA_RUNTIME/SHARED_ATOM_RUNTIME — paths keyed only by commit hash under /it-share, which cluster-configs/mi355x-amds.yaml documents as a filesystem genuinely shared across all nodes (runtime_config_transport: shared-filesystem). These git operations (lines 72-83) run with no lock. Three lines later the container-image staging for the exact same shared filesystem is wrapped in exec 9>${SHARED_IMAGE}.lock; flock -w 2400 9, and the script's own header comment states: 'The shared lock makes concurrent aggregate/disaggregated validations safe.' The author clearly anticipated that agg and disagg stage jobs run concurrently against shared paths and locked the image accordingly — the new git checkout logic added by this PR simply omits the same guard. runners/launch_mi300x-amds-srt.sh's ensure_git_checkout() has the identical gap in its dir-exists branch (unlocked fetch+checkout --detach on the shared REMOTE_INFERA_RUNTIME/REMOTE_ATOM_RUNTIME).\n\nWhy it's real, not theoretical. This PR's own new matrix rows, qwen3-0.6b-fp16-mi355x-atom-infera-srt-agg and -disagg, both target cluster:mi355x-amds and both hardcode the identical INFERA_COMMIT/ATOM_COMMIT, so both resolve to the exact same shared directory. In a full sweep these run as separate parallel GitHub Actions matrix jobs, each independently invoking the launcher and each submitting its own stage sbatch job on the shared cluster. The hardware-validation runs cited in the PR description happened to run agg and disagg in separate dispatches (31523759856 vs 31524668094), which is exactly why the race was never triggered during validation — a real full-sweep matrix run would trigger it.\n\nConcrete failure modes (step-by-step).\n1. Cold cache: both jobs evaluate [[ ! -d \"$SHARED_INFERA_RUNTIME/.git\" ]] as true (no per-job temp dir on this path, unlike the MI300X launcher's clone branch) and both run git clone directly into the same target. The loser hits 'destination path already exists and is not an empty directory' and, under set -euo pipefail, the whole stage sbatch — and therefore the lane — fails.\n2. Warm cache: both jobs run git fetch + git checkout --detach concurrently against the same .git. Git's own index.lock/ref-lock protection turns this into a loud failure for one job ('Unable to create '.git/index.lock': File exists'), or, if a job is killed/cancelled mid-checkout, it can leave a stale index.lock behind in the persistent, commit-keyed directory — poisoning every subsequent run against that commit until it's manually cleaned up, since the dir is never recreated (it's keyed only by commit hash, not by run).\n3. Even without an outright lock error, rev-parse HEAD reads HEAD (an atomic, fast write) without guaranteeing the other process finished writing the worktree/index, so a job can pass the post-checkout test guard while proceeding on a partially-updated worktree.\n\nWhy nothing else prevents this. There is no per-job temporary directory + atomic mv for these git operations on the MI355X path (unlike the image-staging block right above it, and unlike the fresh-clone branch of MI300X's ensure_git_checkout), and no flock guarding the existing-target fetch/checkout branch on either launcher.\n\nFix. Wrap the Infera/ATOM git clone/fetch/checkout --detach sequence in the same flock pattern already used for SHARED_IMAGE (one lock file per commit-keyed target), in both runners/launch_mi355x-amds-srt.sh (lines 72-83) and the else branch of ensure_git_checkout() in runners/launch_mi300x-amds-srt.sh.
| ensure_git_checkout() { | ||
| local target="\$1" | ||
| local repository="\$2" | ||
| local commit="\$3" | ||
| local temporary="\${target}.tmp.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| local quarantine="\${target}.incomplete.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| if [[ ! -d "\$target/.git" ]]; then | ||
| if [[ -e "\$target" ]]; then | ||
| mv "\$target" "\$quarantine" | ||
| fi | ||
| git clone --quiet "\$repository" "\$temporary" | ||
| git -C "\$temporary" fetch --quiet origin "\$commit" | ||
| git -C "\$temporary" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$temporary" rev-parse HEAD)" = "\$commit" | ||
| mv "\$temporary" "\$target" | ||
| else | ||
| git -C "\$target" fetch --quiet origin "\$commit" | ||
| git -C "\$target" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$target" rev-parse HEAD)" = "\$commit" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🟡 In ensure_git_checkout() (runners/launch_mi300x-amds-srt.sh:124-144), when $target exists without a .git dir, the stale copy is moved aside into a unique ${target}.incomplete.${SLURM_JOB_ID}.${BASHPID} quarantine directory, but that directory is never removed — no rm -rf and no cleanup trap exist anywhere in the file. Each time this branch triggers on a given /raid staging node, one more full clone of srt-slurm/Infera/ATOM is stranded permanently; fix by adding rm -rf "$quarantine" once the fresh clone lands (or by dropping the quarantine step entirely and rm -rf "$target" directly, since the stale copy is discarded regardless).
Extended reasoning...
What the bug is. ensure_git_checkout() is a helper defined inline inside the staging srun script in runners/launch_mi300x-amds-srt.sh (lines 124-144). It is used to materialize the pinned srt-slurm, Infera, and ATOM checkouts on the shared /raid staging path for each node. When $target (e.g. REMOTE_SRT_RUNTIME, REMOTE_INFERA_RUNTIME, or REMOTE_ATOM_RUNTIME) exists but is missing its .git directory — the signature of an interrupted or otherwise corrupt prior clone — the function moves it aside with mv "$target" "$quarantine", where quarantine="${target}.incomplete.${SLURM_JOB_ID}.${BASHPID}". It then clones fresh into a temporary directory and atomically mvs that into place as $target.
The code path that triggers it. A grep for quarantine across the whole file returns exactly two hits: the variable declaration and the mv that populates it. There is no rm -rf of $quarantine, and no trap (EXIT or otherwise) that would clean it up later. Because the quarantine name embeds both SLURM_JOB_ID and BASHPID, it is unique per invocation, so nothing ever collides with or overwrites a previous quarantine directory either — each triggering run simply adds one more directory that nothing will ever touch again.
Why existing code doesn't prevent it. The new atomic clone-into-temporary-then-mv pattern this PR introduces makes the missing-.git branch relatively rare in steady state (a target only ends up in that state from an interrupted/corrupt legacy clone, not from a normal successful run). But 'rare' is not 'zero' — flaky CI runs, node preemption mid-clone, or a first-time bring-up against a pre-existing bad target will all trip this branch, and there's no mechanism anywhere in the script, in cron, or elsewhere that reclaims /raid space consumed by these quarantine directories.
Impact. Each occurrence permanently strands a full git clone of srt-slurm, Infera, or ATOM (each a non-trivial checkout) on the shared /raid/hf-hub-cache/inferencex/srt-slurm staging path for that node. Over the life of the cluster, repeated flaky or interrupted runs accumulate these directories with no reclamation path, silently consuming shared disk that other jobs on the same node depend on.
How to fix it. Add rm -rf "$quarantine" right after mv "$temporary" "$target" succeeds, so the stale copy is deleted once the fresh clone is safely in place. An equally valid and simpler alternative is to skip quarantining altogether and just rm -rf "$target" before cloning, since the old copy is being unconditionally discarded either way and there's no code anywhere that reads from the quarantine path afterward.
Step-by-step proof. (1) A prior CI run is killed mid-clone, leaving REMOTE_SRT_RUNTIME on the node as a directory without a .git subdirectory. (2) A later run's staging srun step calls ensure_git_checkout "$srt_runtime" ...; since [[ ! -d "$target/.git" ]] is true and [[ -e "$target" ]] is true, it executes mv "$target" "${target}.incomplete.<job2>.<pid2>". (3) It clones fresh into ${target}.tmp.<job2>.<pid2>, verifies the commit, and mvs it into place as $target. (4) The function returns; the .incomplete.<job2>.<pid2> directory — a full git clone — is left on disk with no reference to it anywhere in the rest of the script. (5) Repeat for a third interrupted run: a second, distinct .incomplete.<job3>.<pid3> directory appears alongside the first, and so on indefinitely.
| ensure_git_checkout() { | ||
| local target="\$1" | ||
| local repository="\$2" | ||
| local commit="\$3" | ||
| local temporary="\${target}.tmp.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| local quarantine="\${target}.incomplete.\${SLURM_JOB_ID}.\${BASHPID}" | ||
| if [[ ! -d "\$target/.git" ]]; then | ||
| if [[ -e "\$target" ]]; then | ||
| mv "\$target" "\$quarantine" | ||
| fi | ||
| git clone --quiet "\$repository" "\$temporary" | ||
| git -C "\$temporary" fetch --quiet origin "\$commit" | ||
| git -C "\$temporary" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$temporary" rev-parse HEAD)" = "\$commit" | ||
| mv "\$temporary" "\$target" | ||
| else | ||
| git -C "\$target" fetch --quiet origin "\$commit" | ||
| git -C "\$target" checkout --quiet --detach "\$commit" | ||
| test "\$(git -C "\$target" rev-parse HEAD)" = "\$commit" | ||
| fi | ||
| } | ||
| ensure_container_image "${ENGINE_SQSH}" "${ENGINE_IMAGE}" | ||
| if [[ -n "${AUX_IMAGE}" ]]; then | ||
| ensure_container_image "${AUX_SQSH}" "${AUX_IMAGE}" | ||
| fi | ||
| git -C "\$srt_runtime" fetch --quiet origin "${SRT_SLURM_COMMIT}" | ||
| git -C "\$srt_runtime" checkout --quiet --detach "${SRT_SLURM_COMMIT}" | ||
| test "\$(git -C "\$srt_runtime" rev-parse HEAD)" = "${SRT_SLURM_COMMIT}" | ||
| make -C "\$srt_runtime" --no-print-directory setup-compute ARCH=x86_64 | ||
| ensure_git_checkout "\$srt_runtime" "${SRT_SLURM_REPOSITORY}" "${SRT_SLURM_COMMIT}" | ||
| make -C "\$srt_runtime" --no-print-directory setup ARCH=x86_64 | ||
| ensure_git_checkout "\$infera_runtime" "${INFERA_REPOSITORY}" "${INFERA_COMMIT}" | ||
| ensure_git_checkout "\$atom_runtime" "${ATOM_REPOSITORY}" "${ATOM_COMMIT}" |
There was a problem hiding this comment.
🟡 The new ensure_git_checkout calls for INFERA_REPOSITORY (cquil11/Infera) and ATOM_REPOSITORY (cquil11/ATOM) run unconditionally in both srt-slurm launchers, so the pre-existing vLLM (mi300x) and SGLang (mi355x) lanes now clone/fetch two personal-fork repos they never use, even though the sibling container-image staging directly above (the [[ -n ${AUX_IMAGE} ]] check on mi300x, the image-derived RUNTIME_IMAGE on mi355x) is already gated by image type. Gate both checkouts on the ATOM_IMAGE branch of the case statement so unrelated lanes don't gain a hard new dependency on availability of two personal fork repos.
Extended reasoning...
The bug: Both runners/launch_mi300x-amds-srt.sh and runners/launch_mi355x-amds-srt.sh add a case statement that resolves the recipe's container image (ENGINE_IMAGE/RUNTIME_IMAGE) and, on mi300x, an auxiliary image (AUX_IMAGE) — and the auxiliary image staging is correctly gated with if [[ -n "${AUX_IMAGE}" ]]; then ensure_container_image ...; fi (mi300x, around line 148) so the vLLM-router container is only imported for VLLM_IMAGE rows. Immediately below that gated block, the PR adds two ensure_git_checkout calls for INFERA_REPOSITORY (https://github.com/cquil11/Infera.git) and ATOM_REPOSITORY (https://github.com/cquil11/ATOM.git) with no equivalent gate — they run for every matrix row regardless of which branch of the case "${IMAGE}" statement matched. The mi355x launcher has the identical problem: the Infera/ATOM clone+fetch+checkout block sits above and outside the case "${IMAGE}" statement entirely, in the sbatch stage script.\n\nCode path that triggers it: configs/amd-master.yaml still routes the pre-existing qwen3-0.6b-fp16-mi300x-vllm-srt-agg/-disagg lanes (VLLM_IMAGE) and qwen3-0.6b-fp16-mi355x-sglang-srt-agg/-disagg plus the production qwen3.5-fp8-mi355x-sglang-srt-disagg migration lane (SGLANG_IMAGE) through these exact launchers. None of these recipes reference /infera-source or /atom-source (only the new ATOM_IMAGE recipes do, via PYTHONPATH: "/atom-source:/infera-source"), yet their staging job now unconditionally runs git clone/fetch/checkout against two personal GitHub fork repos under set -euo pipefail.\n\nWhy nothing else prevents it: The author clearly intended per-image conditional staging — that's exactly what the new case "${IMAGE}" statement and the AUX_IMAGE guard demonstrate — but the Infera/ATOM checkouts were left outside that gate, seemingly by oversight rather than design, since they have no purpose for the vLLM/SGLang code paths.\n\nImpact: Any Slurm staging job for a previously-green vLLM or SGLang srt-slurm lane now fails if github.com/cquil11/Infera or github.com/cquil11/ATOM becomes rate-limited, renamed, made private, or deleted (these are personal forks, not the org's canonical repos) — even though that lane never uses either checkout. It's also wasted staging time: every run clones a full ATOM/Infera fork tree for lanes that will never touch it.\n\nProof (mi300x lane): Take qwen3-0.6b-fp16-mi300x-vllm-srt-agg, which sets image: vllm/vllm-openai-rocm:v0.26.0. In the launcher, IMAGE matches the "$VLLM_IMAGE" case arm, so ENGINE_IMAGE=$VLLM_IMAGE, AUX_IMAGE=$VLLM_ROUTER_IMAGE. The staging script then runs ensure_container_image "${ENGINE_SQSH}" "${ENGINE_IMAGE}", then (since AUX_IMAGE is non-empty) ensure_container_image "${AUX_SQSH}" "${AUX_IMAGE}" — both correctly scoped to vLLM. Immediately after, unconditionally: ensure_git_checkout "$infera_runtime" "${INFERA_REPOSITORY}" "${INFERA_COMMIT}" and ensure_git_checkout "$atom_runtime" "${ATOM_REPOSITORY}" "${ATOM_COMMIT}" run regardless. If github.com/cquil11/Infera returns a 404 or rate-limit at that moment, git clone --quiet fails, the staging srun bash block exits non-zero (due to set -euo pipefail), the sbatch --wait staging job fails, and the entire qwen3-0.6b-fp16-mi300x-vllm-srt-agg benchmark job fails — despite this lane never referencing /infera-source in its recipe YAML.\n\nFix: Move the two ensure_git_checkout calls for Infera/ATOM inside the ATOM_IMAGE branch of each launcher's case statement (or gate them the same way AUX_IMAGE is gated on mi300x), and correspondingly make the /infera-source//atom-source mounts conditional so unrelated vLLM/SGLang lanes don't pick up mount points they never use.
3 participants
Summary
This stacked validation PR exercises the combined AMD platform work plus ATOM backend and Infera routing support from SemiAnalysisAI/srt-slurm#3 on MI300X and MI355X.
It intentionally uses Qwen3-0.6B and short fixed-sequence workloads to validate orchestration, routing, KV transfer, cleanup, and artifact collection rather than tune performance.
Dependency structure
Current runtime pins:
5ecfb13d1ba0960045482f1ef006312d8729d37a8ed8f1728c745d4e91ba9eaa09ed81159aa57e412ab42bc2c64d1ad04f698c396da48473e71a6dbbConfigurations
For both MI300X and MI355X:
The launcher mounts the pinned Infera and ATOM sources over the stable
rocm/infera:atom-v0.1.1image so validation is reproducible without rebuilding the container.Implementation and local validation
bash -n.git diff --checkpasses.EVALSassignment and has a regression test ensuring every emitted matrix variable is defined. This fixes the previously red wrapper where all materialized workload jobs were green.Hardware evidence
MI300X
Run 31524554022, exact tested head
f9bdbe1c583182be2071c99fe0bb22cdcb68efb7:COMPLETED 0:0; two workers registered; c1/c4 succeeded; 25 HTTP 200 responses; no fatal server markers.COMPLETED 0:0; 25/25 producer writes, 25/25 decode first-token injections, two TCP transports, zero RDMA installs, and zero registration/transfer/socket failures.MI355X
COMPLETED 0:0; 25/25 writes, 25/25 decode injections, two TCP transports, zero RDMA installs, zero transfer failures; c1/c4 results and logs uploaded.COMPLETED 0:0; two workers registered; c1/c4 succeeded; artifacts uploaded.The prior MI355X attempt that returned HTTP 200 but failed every KV write is not counted as success. Its root cause was Mooncake selecting RDMA after HCA discovery despite
protocol=tcp. Exact corrected runs injectMC_FORCE_TCP=true; their uploaded logs prove TCP installation and clean KV transfer.Current head
PR head is
eb814bbf0894e17132d43b241e4671388e698ba9. The hardware-tested runtime content is unchanged fromf9bdbe1; the final commit only repairs the E2E matrix output and adds its regression test. Final-head workflow proof: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/31526765208 completed SUCCESS, including matrix generation, the MI300X aggregate workload, result collection, and success-rate calculation.