Skip to content

feat(runtime): add AMD accelerator support - #1

Open
cquil11 wants to merge 8 commits into
mainfrom
agent/amd-multinode-runtime
Open

feat(runtime): add AMD accelerator support#1
cquil11 wants to merge 8 commits into
mainfrom
agent/amd-multinode-runtime

Conversation

@cquil11

@cquil11 cquil11 commented Aug 10, 2026

Copy link
Copy Markdown

Summary

Make the shared srt-slurm runtime usable on AMD/ROCm clusters while retaining the existing backend, frontend, and topology abstractions. Cluster-specific GPU allocation, device binding, filesystem layout, and network assumptions are configured independently of the inference engine.

Review map

  • core/accelerator.py, core/schema.py, core/runtime.py, cli/mixins/worker_stage.py: add accelerator_vendor and vendor-native GPU visibility; preserve NVIDIA defaults and the legacy vLLM binding option.
  • cli/submit.py, templates/job_script_minimal.j2: support GPU allocation via gpus-per-node, gres, or no generated directive; optionally embed resolved recipes and cluster profiles for node-local output layouts; select a staged compute-side source checkout.
  • frontends/static_router.py, frontends/sglang.py: gate slow-starting static SGLang workers before router registration and propagate runtime Slurm options to native routers.
  • core/runtime.py, core/ip_utils, worker/frontend launch paths: resolve control-plane addresses on the configured network and use container-visible log/config paths.
  • cli/do_sweep.py, Makefile, submit-time setup validation: preserve configured Hugging Face caches and environments; separate compute setup from Dynamo control-plane requirements.
  • core/slurm.py, submit CLI: provide srtctl wait to stream logs and report terminal Slurm accounting status without downstream polling or host-repair hooks.
  • Shared-runtime changes also include writable job-local Dynamo version installs, failure-log decoding, and runtime package provenance. The Dynamo install changes affect the NVIDIA path and warrant independent review; this PR does not validate Dynamo on ROCm.

Stack and scope

GitHub native stack: #1 (shared AMD runtime) → #11 (vLLM MoRI-IO) → #7 (ATOM/AToMesh).

MoRI-IO connector generation, ZMQ discovery, dynamic-discovery health probing, MoRI-specific port handling, and their tests/documentation live in #11. ATOM/AToMesh and its Mooncake integration live in #7. The linear stack order does not mean ATOM depends on the MoRI engine integration.

The shared InferenceX adapter and legacy-launcher removal are reviewed in InferenceX #2542. Benchmark workloads remain downstream through the custom benchmark interface.

Validation


Note

Medium Risk
Changes Slurm script generation, multi-node networking, and job bootstrap paths (embedded configs, Dynamo overlay installs); mistakes could break allocations or control-plane reachability on non-NVIDIA clusters, though behavior on shared-filesystem NVIDIA defaults is largely preserved.

Overview
Extends the shared Slurm runtime for AMD/ROCm and heterogeneous cluster layouts without changing the core backend/frontend model. Cluster profile gains accelerator_vendor, gpu_sbatch_directive (gpus-per-node / gres / none), and runtime_config_transport: in embedded mode the batch script base64-decodes the resolved recipe plus srtslurm.yaml on the head node, bootstraps node-local logs, and sets SRTSLURM_CONFIG so login and compute nodes need not share the same paths.

GPU binding is vendor-neutral: vLLM adds set_visible_devices (legacy set_cuda_visible_devices kept); workers set ROCR_VISIBLE_DEVICES or CUDA_VISIBLE_DEVICES via a new accelerator helper. Control-plane connectivity uses infra_node_ip / head_node_ip on the configured network_interface, with IP discovery preferring private hostname -I addresses over the default route.

Operational UX: make setup-compute installs arch-matched uv only; submit validation skips NATS/etcd for direct/static frontends (vllm, vllm-router, sglang, trtllm_serve). New srtctl wait polls Slurm accounting, optionally tails a shared sweep log, and exits with the job status. Hugging Face prefetch honors HF_HUB_CACHE / HUGGINGFACE_* env and keeps the srun bash wrapper so cache env is not dropped.

Launch reliability: infra/worker config dumps and head setup use container-mounted log paths; static routers (notably SGLang) wait for backend HTTP health before registration; runtime srun_options propagate to native routers. Stable Dynamo pip releases install with --no-deps into a job-local /logs site-packages overlay (wheel/source paths unchanged). Docs cover vllm-router, host setup scripts, and srtctl wait.

Reviewed by Cursor Bugbot for commit 0ebf722. Bugbot is set up for automated code reviews on this repo. Configure here.

@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 3 potential issues.

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 d7b483c. Configure here.

Comment thread src/srtctl/cli/submit.py
Comment thread src/srtctl/backends/vllm.py Outdated
Comment thread src/srtctl/core/schema.py
raise ValidationError(
f"sglang_config.{mode}: tp-size={tp_size} must be divisible by "
f"dp-size={dp_size}; SGLang rejects this data-parallel layout"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SGLang DP validator never runs

Low Severity

_validate_sglang_data_parallelism is new dead code: nothing calls it, including SrtConfig.__post_init__. Invalid SGLang tp-size/dp-size layouts are not rejected at load or dry-run and still fail only after workers start.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d7b483c. Configure here.

@cquil11

cquil11 commented Sep 3, 2026

Copy link
Copy Markdown
Author

Cleanup pushed in 93b854c.

  • Remove the recipe-level host_setup_script hook and its non-container host srun.
  • Add platform-neutral srtctl wait JOB_ID --log-file PATH: stream the allocation log, tolerate queue/accounting lag and transient query timeouts, and propagate the terminal allocation exit status. It never cancels or resubmits jobs.
  • InferenceX now calls native apply --json and wait; the adapter does not manage Slurm or repair hosts.

Validation: 249 focused base runtime tests passed. On the rebased ATOM child, 396 runtime/backend/router/config/CLI tests plus 76 benchmark tests passed. Three existing benchmark shell tests fail on a missing profiling.sh path on this macOS environment; the same failures were reproduced on the old runtime. Read-only live Slurm accounting checks correctly returned COMPLETED/0 for job 42158 and FAILED/1 for job 42154.

ATOM PR #7 is rebased on this head. The already-running InferenceX ATOM sweep remains pinned to its original runtime; it is not validation of this cleanup.

@cquil11

cquil11 commented Sep 4, 2026

Copy link
Copy Markdown
Author

MiniMax-M3 MI355X native-vLLM validation exposed a real orchestration defect in run https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33836289639: all four TP children inherited the same VLLM_PORT scan base, and MoRI listeners collided before serving started. The unmodified upstream vLLM 0.28.0 port allocator reproduces the collision with that environment; without it, the kernel allocates ephemeral ports.

Fixed in c0472a8: MoRI keeps upstream ephemeral-port allocation; non-MoRI rendezvous bases remain unchanged. No engine source patch, custom image, cleanup helper, or model-option change. Two regression cases (backend connector and role override) fail before the fix and pass afterward. PR #7 incorporates the fix at 33a3d81.

112 focused vLLM/router/ATOM/accelerator/dry-run tests pass on the combined runtime. The broad local suite reports 1546 passed, 2 skipped, 6 deselected, and 5 failures; all five reproduce on the exact pre-change 81d4627 baseline (three sa-bench shell-path tests, one local shell-profile test, and one Linux-only CPU-affinity mock on macOS). Previously validated InferenceX recipes remain pinned to 81d4627; only the archived MiniMax compatibility lane selects the new runtime.

@cquil11

cquil11 commented Sep 4, 2026

Copy link
Copy Markdown
Author

The MoRI native-port fix in c0472a8 is now validated on MI355X with official vLLM ROCm v0.28.0 and the official vLLM Router nightly, through the combined PR #7 runtime 33a3d81.

Complete InferenceX run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33837838363 — all jobs green, attempt 1. Port PR: SemiAnalysisAI/InferenceX#2824.

1P1D TP4/TP4: all nine 8k/1k concurrency points, 5,116/5,116 measured requests, plus full 1,319-question GSM8K (94.69% strict; unchanged threshold 90%). Per-point and per-sample artifact audits passed, including zero empty eval responses. An earlier independent full throughput sweep on the same runtime also passed. Both allocations cleaned up normally.

No engine source patches or graph-mode/config workarounds were needed. The source fix only preserves native ephemeral-port allocation for MoRI TP children; existing non-MoRI port behavior remains unchanged and covered by the focused regression tests. Previously validated InferenceX recipe pins were not changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant