Skip to content

vllm_dissag: unify 3 launchers into one two-axis driver + models.yaml#171

Open
raviguptaamd wants to merge 5 commits into
ROCm:developfrom
raviguptaamd:vllm-disagg-unified-launcher
Open

vllm_dissag: unify 3 launchers into one two-axis driver + models.yaml#171
raviguptaamd wants to merge 5 commits into
ROCm:developfrom
raviguptaamd:vllm-disagg-unified-launcher

Conversation

@raviguptaamd

@raviguptaamd raviguptaamd commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Two things, stacked:

  1. Consolidates the three vLLM disaggregated-inference launchers in scripts/vllm_dissag/
    (vllm_disagg_server.sh, vllm_disagg_mori_ep.sh, vllm_disagg_server_deepep.sh) into a single
    launcher
    vllm_disagg.sh driven by two orthogonal axes + a validated EP backend.
  2. Ports the MAD-private #324 DeepSeek-V3 MoRI-EP recipe into that consolidated format (connector +
    models.yaml + Dockerfile), so MoE serves through the same launcher — not a parallel script.

Axis model

WIDE_EP=0 (TP) WIDE_EP=1 (wide expert-parallel)
CONNECTOR=rixl (NixlConnector) dense / TP DeepEP (EP_BACKEND=deepep)
CONNECTOR=moriio (MoRIIOConnector) MoRIIO + TP (new) MoRI-EP (EP_BACKEND=mori)
  • Connector-specific logic → connectors/{rixl,moriio}.sh
  • TP-vs-wideEP fork → parallelism.sh
  • Per-model CLI flags + env overrides → models.yaml catalog (replaces inline declare -A maps / hardcoding)

Adding a model is data-only (edit models.yaml + the slurm allowlist). Invalid connector/EP pairings
(moriio+deepep, rixl+mori) abort with a clear error.

Code changes

Launcher consolidation

  • New vllm_disagg.sh — single driver: axis resolution + validation, topology math, models.yaml
    parse, NODE_RANK role branch (prefill/decode × master/child), container barrier, proxy, benchmark,
    cleanup. DRY_RUN=1 echoes the assembled vllm serve argv for offline parity.
  • New connectors/rixl.sh (NixlConnector: TP + DeepEP) and connectors/moriio.sh
    (MoRIIOConnector: MoRIIO+TP + MoRI-EP). Each implements the connector hook contract
    (connector_init, connector_setup_env, connector_runtime_patch, connector_launch_worker,
    connector_wait_workers_ready, connector_start_proxy).
  • New parallelism.sh — shared TP-vs-wideEP helpers.
  • New models.yaml — per-model flag/env catalog (sglang schema; tp/dp blocks selected by WIDE_EP).
  • Deleted the 3 legacy launchers.
  • New tests/parity_check.sh + tests/golden/ — dry-run argv parity gate (byte-identical to the
    3 legacy launchers for every connector × parallelism × role cell, plus validation-rejection checks).
  • New ARCHITECTURE.md — component + state diagrams. New tests/TEST_PLAN.md.

#324 DeepSeek-V3 MoRI-EP port

  • connectors/moriio.sh: per-role mori all2all (mori_high_throughput/mori_low_latency);
    --block-size ${KV_BLOCK_SIZE}, --kv-cache-memory-bytes, VLLM_ROCM_USE_AITER_MLA override;
    MoRI fabric env (MORI_RDMA_TC/SL, MORI_SHMEM_HEAP_SIZE); cudagraph NONE emitted as
    compilation-config cudagraph_mode:NONE +quant_fp8 (never bare --enforce-eager, which crashes
    engine init on these AITER images); per-role PREFILL/DECODE_CUDAGRAPH_MODE.
  • Runtime patch: switched to the idempotent Python patcher apply_39276_rebased.py (new) with
    topology-defaulted SKIP_RUNTIME_PATCH (explicit value wins; images that bake the fixes set =1).
  • Proxy: connector_start_proxy supports vllm_router (ROUTER_BINARY/PATH + --kv-connector moriio + discovery address + registration gate) and moriio_toy (with online_serving/
    disaggregated/ path resolution). moriio defaults to vllm_router — the toy proxy cannot route
    the wideEP DP-rank KV-notify. Sets ROUTER_PORT (default 30000) so the router gets a valid --port.
  • models.yaml: DeepSeek-V3 / R1 / V3-5layer env: blocks (YAML anchor) encode the validated recipe
    (block=16, MLA off, kv-cache-memory-bytes, per-role cudagraph + mori backends, SKIP_RUNTIME_PATCH=1,
    fabric tuning), so MODEL_NAME=DeepSeek-V3 WIDE_EP=1 works without a wrapper.
  • New docker/vllm_disagg_mori_ep_fullsource.ubuntu.amd.Dockerfile — buildable MoRI-EP image
    (MoRI 1db01d8, AITER 0.1.14, vLLM fork b10a9f7a, vllm-router #181 + DP-rank dpfix) on a named
    nightly base, with build-time integrity asserts. The public-base vllm_disagg_inference Dockerfile is
    kept unchanged for dense models.
  • New helper scripts benchmark_long_context.sh, benchmark_parser.py.
  • run_xPyD_models.slurm: BENCHMARK_SCRIPT selector (sweep/long_context); -e plumbing for the
    recipe/cache/proxy env; cache-dir env forwarded only-if-set so a prewarmed image's baked cache wins;
    driver runs $BENCHMARK_SCRIPT_FILE; apply_39276_rebased.py added to REQUIRED_FILES.

Back-compat

RUN_MORI=1 / RUN_DEEPEP=1 still work (mapped to the new axes); no-flags default stays rixl + TP.

Testing

  • tests/parity_check.sh — byte-identical argv vs the 3 legacy launchers (offline, no GPUs).
  • moriio wideEP argv verified byte-identical to #324's launcher.

Notes / follow-ups

  • MoE models require a co-versioned AITER/vLLM image (the fullsource Dockerfile); on a mismatched image
    they fail inside AITER's MoE GEMM path at engine init — an image concern, independent of the launcher.
  • The fullsource base is a private rocm/pytorch-private nightly (BASE_IMAGE overridable).
  • VLLM_CACHE_PERSIST (image-digest-keyed host JIT cache, to avoid per-run cold AITER/MoRI compiles)
    is a planned follow-up, not in this PR.
  • #325 Hy3 (GQA) support is deferred.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 24, 2026 04:08

Copilot AI left a comment

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.

Pull request overview

This PR consolidates the vLLM disaggregated prefill/decode launch flow in scripts/vllm_dissag/ by replacing three legacy launcher scripts with a single two-axis driver (CONNECTOR × WIDE_EP) plus a models.yaml catalog for per-model flags/env, and adds an offline parity test gate to keep the assembled vllm serve argv consistent with legacy behavior.

Changes:

  • Replace vllm_disagg_{server,mori_ep,server_deepep}.sh with a unified vllm_disagg.sh that sources connector + parallelism profiles and reads per-model config from models.yaml.
  • Update run_xPyD_models.slurm to resolve axes/back-compat shims and plumb new env vars into Docker runs.
  • Add tests/parity_check.sh + golden fixtures to enforce byte-identical argv parity vs legacy launchers (plus validation rejection checks).

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
scripts/vllm_dissag/vllm_disagg.sh New unified launcher with axis resolution, role branching, YAML model config parsing, and DRY_RUN support.
scripts/vllm_dissag/connectors/rixl.sh New connector profile implementing rixl TP + DeepEP wideEP behavior.
scripts/vllm_dissag/connectors/moriio.sh New connector profile implementing MoRI-EP wideEP and new MoRIIO+TP path.
scripts/vllm_dissag/parallelism.sh Shared helper for wideEP master/child role args.
scripts/vllm_dissag/models.yaml New model catalog replacing inline associative arrays / hardcoding.
scripts/vllm_dissag/tests/TEST_PLAN.md New before/after validation plan documenting parity + live matrix.
scripts/vllm_dissag/tests/parity_check.sh New offline parity gate that diffs DRY_RUN argv vs goldens.
scripts/vllm_dissag/tests/golden/*.txt New golden argv fixtures captured from legacy launchers.
scripts/vllm_dissag/tests/golden/gen_golden.sh Utility to regenerate golden fixtures.
scripts/vllm_dissag/run_xPyD_models.slurm Updated slurm entry to use unified launcher and axis/back-compat resolution.
scripts/vllm_dissag/README.MD Updated docs to match the unified launcher + models.yaml + test flow.
scripts/vllm_dissag/vllm_disagg_server.sh Deleted legacy rixl TP launcher.
scripts/vllm_dissag/vllm_disagg_mori_ep.sh Deleted legacy MoRI-EP launcher.
scripts/vllm_dissag/vllm_disagg_server_deepep.sh Deleted legacy DeepEP launcher.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +7 to +8
# CONNECTOR = rixl | moriio (KV transfer; default moriio)
# WIDE_EP = 0 (TP) | 1 (wideEP) (parallelism; default per back-compat shim)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: header comment now reads "default rixl via back-compat shim" to match the axis-resolution logic (and the README).

Comment on lines +31 to +32
SCRIPT_DIR="${NIXL_COOKBOOK_PATH:-$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

SCRIPT_DIR already falls back to the script's own directory and NIXL_COOKBOOK_PATH is always set by the slurm's docker -e; the ls line is now guarded/quoted. Left the default resolution as-is.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +86 to +87
echo "Listing NIXL_COOKBOOK_PATH : "
ls ${NIXL_COOKBOOK_PATH}
Comment on lines +186 to +190
--no-enable-prefix-caching \
--all2all-backend "${_all2all}" \
--trust-remote-code \
--distributed-timeout-seconds "${DISTRIBUTED_TIMEOUT_SECONDS:-7200}" \
"${exec_args[@]}" "${extra_args[@]}" "${kv_args[@]}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not an issue: the moriio wideEP path does append model_args to the vllm serve invocation (see the "${model_args[@]}" in both the DRY_RUN and live branches). The dp: blocks are empty today so it's a no-op, but the wiring is present.

Comment on lines +209 to +212
"${exec_args[@]}" \
"${extra_args[@]}" \
"${kv_args[@]}" \
2>&1 | tee /run_logs/${SLURM_JOB_ID}/${log_prefix}_NODE${NODE_RANK}.log >/dev/null &

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As above — model_args is appended in the non-DRY_RUN wideEP launch too.

Comment on lines +309 to +312
--all2all-backend "${backend}" \
${DBO_ARGS} \
"${extra_args[@]}" \
--kv-transfer-config "${kv_config}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: model_args now appended in the deepep DRY_RUN argv (empty today, parity unchanged).

Comment on lines +330 to +334
--all2all-backend "${backend}" \
${DBO_ARGS} \
"${extra_args[@]}" \
--kv-transfer-config "${kv_config}" \
2>&1 | tee /run_logs/${SLURM_JOB_ID}/${log_prefix}_NODE${NODE_RANK}.log >/dev/null &

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: model_args now appended in the non-DRY_RUN deepep launch.

Comment thread scripts/vllm_dissag/models.yaml Outdated
Comment on lines +16 to +18
# BOUNDARY (do NOT put these here — the launcher/connector owns them):
# --tensor-parallel-size / --data-parallel-size / --enable-expert-parallel /
# --all2all-backend / --kv-transfer-config / --port / transfer backend.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: reworded the BOUNDARY note. It now states the connector owns the wideEP degree/transport, while the tp: blocks DO carry --tensor-parallel-size (reproducing the legacy declare -A maps); moriio+TP strips the duplicate, rixl+TP takes the degree from the yaml.

export BENCHMARK_CON="8 16 32"
export BENCHMARK_COMBINATIONS="1024/1024 8192/1024"
sbatch -N 2 -n 2 run_xPyD_models.slurm
python3 benchmark_parser.py <log_path>/benchmark_XXX_CONCURRENCY.log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

benchmark_parser.py does ship in this directory (added alongside parse_to_csv.py), so the README command is valid.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
MASTER_PORT="${MASTER_PORT:-23731}"
NODE_RANK="${NODE_RANK:-0}"
NNODES="${NNODES:-1}"
MODEL_PATH=$MODEL_PATH
@raviguptaamd raviguptaamd force-pushed the vllm-disagg-unified-launcher branch from 01b1650 to 455e2b4 Compare July 1, 2026 06:13
Copilot AI review requested due to automatic review settings July 1, 2026 06:13

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +7 to +9
# CONNECTOR = rixl | moriio (KV transfer; default moriio)
# WIDE_EP = 0 (TP) | 1 (wideEP) (parallelism; default per back-compat shim)
# EP_BACKEND= mori | deepep (only when WIDE_EP=1; default = connector partner)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: header comment now reads "default rixl via back-compat shim" to match the axis-resolution logic (and the README).

Comment on lines +165 to +170
import os, yaml, shlex
m = yaml.safe_load(open(os.environ["MODELS_YAML"])) or {}
cfg = m.get(os.environ["MODEL_NAME"]) or {}
for k, v in (cfg.get("env") or {}).items():
print(f'export {k}={shlex.quote(str(v))}')
PY

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed: the models.yaml env: generator now skips any key already in the environment, so a submit-time docker -e VAR=... overrides the yaml value. Precedence is now connector default < models.yaml < submit -e, matching the docs. Verified with a submit-time override test.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
MASTER_PORT="${MASTER_PORT:-23731}"
NODE_RANK="${NODE_RANK:-0}"
NNODES="${NNODES:-1}"
MODEL_PATH=$MODEL_PATH

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: replaced the no-op with : "${MODEL_PATH:?...}" so an unset MODEL_PATH fails fast with a clear message. (Note: this script doesn't run under set -u, but the validation is a genuine improvement.)

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
_wait_log_signal_or_fail() {
local LOG_FILE="$1" LABEL="$2" SEARCH_SIGNAL="$3" TIMEOUT_SECONDS="$4" SLEEP_SECONDS="$5"
local ELAPSED=0
until grep -q "${SEARCH_SIGNAL}" "${LOG_FILE}" 2>/dev/null; do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: changed to grep -Fq (fixed-string) so the trailing '.' in "Application startup complete." can't act as a wildcard.

Comment thread scripts/vllm_dissag/README.MD Outdated
Comment on lines +161 to +162
`PROXY_TYPE` (rixl connector only): `vllm_router` (default, Rust load balancer) or `toy_proxy`
(Python, testing). The `moriio` connector uses its own fixed proxy (`moriio_toy_proxy_server.py`).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the README proxy section now documents PROXY_TYPE=vllm_router (default) and the per-connector toy proxy, and the port table reflects ROUTER_PORT (30000) for the default router.

Comment on lines +84 to +99
def extract(pattern):
m = re.search(pattern, run)
return float(m.group(1).replace(',', '')) if m else None

results.append({
'Model': model,
'xP_yD': xp_yd,
'ISL': isl,
'OSL': osl,
'Concurrency': concurrency,
'Prompts': prompts,
'Successful': extract(r'Successful requests:\s+([\d,]+)'),
'Failed': extract(r'Failed requests:\s+([\d,]+)'),
'Total_Input_Tokens': extract(r'Total input tokens:\s+([\d,]+)'),
# vLLM labels decode tokens "Total generated tokens".
'Total_Output_Tokens': extract(r'Total generated tokens:\s+([\d,]+)'),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: extract() now returns int for whole-number values (counters, token totals) and float otherwise, so Successful/Failed no longer render as 16.0 in the table or CSV.

Copilot AI review requested due to automatic review settings July 2, 2026 04:54

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
# ONE launcher for all KV connectors and parallelism modes. Two axes select the
# behavior; the all-to-all EP backend is validated against the connector:
#
# CONNECTOR = rixl | moriio (KV transfer; default moriio)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: header comment now reads "default rixl via back-compat shim" to match the axis-resolution logic (and the README).

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
MASTER_PORT="${MASTER_PORT:-23731}"
NODE_RANK="${NODE_RANK:-0}"
NNODES="${NNODES:-1}"
MODEL_PATH=$MODEL_PATH
Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +86 to +87
echo "Listing NIXL_COOKBOOK_PATH : "
ls ${NIXL_COOKBOOK_PATH}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: now [[ -n "${NIXL_COOKBOOK_PATH:-}" ]] && ls "${NIXL_COOKBOOK_PATH}", and the echo prints the resolved path (or ).

Comment on lines +65 to +80
export NCCL_SOCKET_IFNAME="${NCCL_SOCKET_IFNAME:-eth0}"
export VLLM_USE_V1=1
export VLLM_SERVER_DEV_MODE=0
export VLLM_NIXL_SIDE_CHANNEL_HOST="${host_ip}"
export VLLM_NIXL_SIDE_CHANNEL_PORT=5557
export UCX_TLS=rc,sm,self,rocm_copy,rocm_ipc,tcp
export UCX_SOCKADDR_TLS_PRIORITY=rdmacm,tcp
export UCX_SOCKADDR_CM_ENABLE=y
export UCX_RDMA_CM_ENABLED=y
export UCX_MEMTYPE_CACHE=y
export UCX_RNDV_SCHEME=get_zcopy
export UCX_RNDV_THRESH=4k
export UCX_ROCM_IPC_MIN_ZCOPY=0
export HSA_ENABLE_SDMA=1
export UCX_LOG_LEVEL=info
export NIXL_LOG_LEVEL=DEBUG

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Won't change: these TP env exports are byte-identical to the legacy vllm_disagg_server.sh, and the parity gate (tests/parity_check.sh + goldens) asserts that. The per-model env: layering applies to model-tuning vars, not the connector's fabric env. Converting these to ${VAR:-} would break the parity goldens whose purpose is faithful reproduction of the legacy launcher.

except PackageNotFoundError: pass
return None
av = get(("amd-aiter", "amd_aiter", "aiter"))
assert av and av.split("+", 1)[0] == "0.1.14", f"AITER downgraded: {av!r}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — assertion updated to 0.1.16.post3 to match the installed AITER wheel.

Comment on lines +216 to +220
echo "Error: DOCKER_IMAGE_NAME is not set. Please export DOCKER_IMAGE_NAME before running." >&2
echo " There is no public prebuilt image. Build your own from the provided Dockerfile:" >&2
echo " docker build -f docker/vllm_disagg_mori_ep_fullsource.ubuntu.amd.Dockerfile \\" >&2
echo " -t <your-registry>/vllm-disagg-mori-ep:local ." >&2
echo " then: export DOCKER_IMAGE_NAME=<your-registry>/vllm-disagg-mori-ep:local" >&2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the error now also points at docker/vllm_disagg_inference.ubuntu.amd.Dockerfile for the rixl/NIXL path, alongside the mori_ep_fullsource image.

Copilot AI review requested due to automatic review settings July 2, 2026 07:23

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
# ONE launcher for all KV connectors and parallelism modes. Two axes select the
# behavior; the all-to-all EP backend is validated against the connector:
#
# CONNECTOR = rixl | moriio (KV transfer; default moriio)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: header comment now reads "default rixl via back-compat shim" to match the axis-resolution logic (and the README).

Comment on lines +175 to +192
eval "$(python3 - <<'PY'
import os, shlex, yaml
m = yaml.safe_load(open(os.environ["MODELS_YAML"])) or {}
name = os.environ["MODEL_NAME"]; mode = os.environ["PARALLEL_MODE"]
cfg = m.get(name)
if cfg is None:
print(f'echo "WARN: model {name} not in models.yaml; using empty flags"'); raise SystemExit(0)
prefill = cfg.get("prefill") or {}; decode = cfg.get("decode") or {}
def compose(role):
return " ".join(x for x in [
cfg.get("base_flags",""), cfg.get(f"{mode}_flags",""),
(role.get(mode,"") if isinstance(role,dict) else ""),
cfg.get("experimental_flags",""),
] if x).strip()
print(f'MODEL_CONFIG_PREFILL={shlex.quote(compose(prefill))}')
print(f'MODEL_CONFIG_DECODE={shlex.quote(compose(decode))}')
PY
)"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the WARN is now emitted to stderr from Python instead of via an echo passed to eval. (MODEL_NAME is also allowlist-validated in the slurm before reaching here, so this is defense-in-depth.)

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +86 to +87
echo "Listing NIXL_COOKBOOK_PATH : "
ls ${NIXL_COOKBOOK_PATH}
Comment thread scripts/vllm_dissag/models.yaml Outdated
Comment on lines +16 to +18
# BOUNDARY (do NOT put these here — the launcher/connector owns them):
# --tensor-parallel-size / --data-parallel-size / --enable-expert-parallel /
# --all2all-backend / --kv-transfer-config / --port / transfer backend.
except PackageNotFoundError: pass
return None
av = get(("amd-aiter", "amd_aiter", "aiter"))
assert av and av.split("+", 1)[0] == "0.1.14", f"AITER downgraded: {av!r}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: the post-install assertion now checks 0.1.16.post3 (matching the installed wheel); it previously asserted the stale 0.1.14 and would have failed the build.

Copilot AI review requested due to automatic review settings July 3, 2026 00:22
@raviguptaamd raviguptaamd force-pushed the vllm-disagg-unified-launcher branch from ea8f7a7 to fd31466 Compare July 3, 2026 00:24

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
# ONE launcher for all KV connectors and parallelism modes. Two axes select the
# behavior; the all-to-all EP backend is validated against the connector:
#
# CONNECTOR = rixl | moriio (KV transfer; default moriio)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: header comment now reads "default rixl via back-compat shim" to match the axis-resolution logic (and the README).

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
MASTER_PORT="${MASTER_PORT:-23731}"
NODE_RANK="${NODE_RANK:-0}"
NNODES="${NNODES:-1}"
MODEL_PATH=$MODEL_PATH
Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
Comment on lines +86 to +88
echo "Listing NIXL_COOKBOOK_PATH : "
ls ${NIXL_COOKBOOK_PATH}

Comment thread scripts/vllm_dissag/benchmark_parser.py Outdated
Comment on lines +84 to +86
def extract(pattern):
m = re.search(pattern, run)
return float(m.group(1).replace(',', '')) if m else None
Comment on lines +571 to +574
${PREFILL_CUDAGRAPH_MODE:+-e PREFILL_CUDAGRAPH_MODE=$PREFILL_CUDAGRAPH_MODE} \
${DECODE_CUDAGRAPH_MODE:+-e DECODE_CUDAGRAPH_MODE=$DECODE_CUDAGRAPH_MODE} \
${CUDAGRAPH_CAPTURE_SIZES:+-e CUDAGRAPH_CAPTURE_SIZES="$CUDAGRAPH_CAPTURE_SIZES"} \
${MORI_RDMA_TC:+-e MORI_RDMA_TC=$MORI_RDMA_TC} \
Copilot AI review requested due to automatic review settings July 3, 2026 00:26

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
# ONE launcher for all KV connectors and parallelism modes. Two axes select the
# behavior; the all-to-all EP backend is validated against the connector:
#
# CONNECTOR = rixl | moriio (KV transfer; default moriio)
# DRY_RUN=1 echoes each worker's assembled `vllm serve` argv instead of running.
# =============================================================================

SCRIPT_DIR="${NIXL_COOKBOOK_PATH:-$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)}"
@raviguptaamd raviguptaamd force-pushed the vllm-disagg-unified-launcher branch 4 times, most recently from 8557ef2 to 69ef604 Compare July 3, 2026 01:39
Copilot AI review requested due to automatic review settings July 3, 2026 01:39

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.

Comment on lines +31 to +32
SCRIPT_DIR="${NIXL_COOKBOOK_PATH:-$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)}"

Comment thread scripts/vllm_dissag/vllm_disagg.sh Outdated
MASTER_PORT="${MASTER_PORT:-23731}"
NODE_RANK="${NODE_RANK:-0}"
NNODES="${NNODES:-1}"
MODEL_PATH=$MODEL_PATH
Comment on lines +165 to +169
import os, yaml, shlex
m = yaml.safe_load(open(os.environ["MODELS_YAML"])) or {}
cfg = m.get(os.environ["MODEL_NAME"]) or {}
for k, v in (cfg.get("env") or {}).items():
print(f'export {k}={shlex.quote(str(v))}')
Comment on lines +59 to +62
export VLLM_LOGGING_LEVEL=INFO
export VLLM_USE_V1=1
export VLLM_ALL2ALL_BACKEND=mori

| MoRI EP | 20005 | 10001 (fixed, MoRI-specific) |
| Connector | vLLM server port | Proxy port |
|-----------|------------------|------------|
| `rixl` (TP or DeepEP) | 2584 | `ROUTER_PORT` (30000 default) |
@raviguptaamd raviguptaamd force-pushed the vllm-disagg-unified-launcher branch from 69ef604 to 4ab0103 Compare July 3, 2026 01:49
… MoRI-EP support

Consolidate the three standalone vLLM disaggregated-inference launchers into a single
launcher driven by two axes, and add the DeepSeek-V3 MoRI-EP recipe from MAD-private #324.

Launcher
- vllm_disagg.sh: one driver selecting behavior by CONNECTOR={rixl|moriio} x
  WIDE_EP={0=TP|1=wideEP}, with EP_BACKEND={mori|deepep} validated against the connector
  (moriio->mori, rixl->deepep; cross-pairs rejected). Legacy RUN_MORI / RUN_DEEPEP map on
  via a back-compat shim.
- connectors/{rixl,moriio}.sh: per-connector hook implementations. rixl reproduces the
  legacy NixlConnector TP + DeepEP launchers byte-for-byte; moriio provides MoRIIO+TP (new)
  and MoRI-EP wideEP (from #324). parallelism.sh holds the TP-vs-wideEP helpers.
- models.yaml: per-model flags + env catalog (replaces the inline declare -A maps). DeepSeek
  family is wideEP-only (TP rejected in the slurm gate). Deletes the 3 legacy launchers.

Correctness fixes (live-validated on the ROCm 7.2.3 / mori121 image)
- GPU-RDMA registration errno-14: ROCm 7.2.3 can't dmabuf-export HIP-VMM memory, so the KV
  cache must use expandable_segments:False (+ HSA_ENABLE_IPC_MODE_LEGACY=0). This platform
  env lives in per-connector connectors/<CONNECTOR>.env and is forwarded by the slurm via
  docker -e (must reach PID 1; PyTorch reads alloc-conf at import).
- moriio+TP router: --intra-node-data-parallel-size = 1 for TP (was DP_PARALLEL_SIZE_LOCAL,
  which round-robined to nonexistent DP ranks -> data_parallel_rank out of range 500s).
- Llama-70B TP: --disable-custom-all-reduce (avoids a hipIpcGetMemHandle HIP-IPC crash).
- rixl connector honors ROUTER_BINARY (images without a baked vllm-router).

No runtime patcher
- The MoRIIO disagg fixes (#39276 notify, #41751 LL split, DP-rank hash-failsafe) are
  committed in-source in the vLLM the image builds from (public fork, Dockerfile VLLM_REF),
  so connector_runtime_patch is a no-op. Removed the runtime .py patchers
  (apply_39276_rebased.py, apply_moriio_2pd_patches.sh) and the SKIP_RUNTIME_PATCH gate —
  they were a drifting duplicate of fixes that already live upstream in the fork.

Docker
- docker/vllm_disagg_mori_ep_fullsource...Dockerfile: from-source image on the open
  rocm/vllm-dev ci_base — MoRI v1.2.1, AITER 0.1.16.post3, vLLM 06_29 wide-ep WRITE branch,
  vllm-router built from vllm-project/router PR#181 (DP-rank round-robin + 2P2D KV-notify
  dpfix). No public prebuilt image; DOCKER_IMAGE_NAME is BYO-build.

Tests + docs
- tests/: offline gates (gate_check combo allow/reject, argv_assert flag/env checks,
  parity_check golden argv diff vs the legacy launchers) + generic interactive drivers.
- README / ARCHITECTURE / TEST_PLAN: combo matrix, per-model x combo enablement, diagrams.

Validated live (ISL/OSL 512/512 and 1024/1024, plus 8192/1024 & 28000/1024 stress; 0 failed
requests): moriio+TP8 (Llama-70B), moriio+wideEP DeepSeek-V3 1P/1D EP8 and 2P/2D EP16.
rixl+TP and rixl+deepep assemble correct argv but need a NIXL/DeepEP-validated image
(documented in the README image-capability table).

Co-Authored-By: Claude <noreply@anthropic.com>
@raviguptaamd raviguptaamd force-pushed the vllm-disagg-unified-launcher branch from 4ab0103 to c4aff32 Compare July 3, 2026 04:36
Copilot AI review requested due to automatic review settings July 3, 2026 04:36

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Comment on lines +218 to +222
--no-enable-prefix-caching \
--all2all-backend "${_all2all}" \
--trust-remote-code \
--distributed-timeout-seconds "${DISTRIBUTED_TIMEOUT_SECONDS:-7200}" \
"${exec_args[@]}" "${extra_args[@]}" "${kv_args[@]}"
Comment on lines +242 to +245
"${exec_args[@]}" \
"${extra_args[@]}" \
"${kv_args[@]}" \
2>&1 | tee /run_logs/${SLURM_JOB_ID}/${log_prefix}_NODE${NODE_RANK}.log >/dev/null &
Comment on lines 422 to 425
NIXL_COOKBOOK_PATH="/opt/nixl-vllm-cookbook"
BENCHMARK_ITR="${BENCHMARK_ITR:-1}"
BENCHMARK_CON="${BENCHMARK_CON:-}"
BENCHMARK_COMBINATIONS="${BENCHMARK_COMBINATIONS:-}"
raviguptaamd and others added 2 commits July 3, 2026 10:00
Fold the MoRI-EP fullsource stack and the UCX/RIXL/rocSHMEM/DeepEP transport
layer into the single vllm_disagg_inference Dockerfile, gated by WITH_NIXL
(default 1 = all connectors). One image now serves moriio TP + MoRI-EP wideEP
(incl. full DeepSeek-V3) and the rixl NIXL/DeepEP paths. Built + validated on
ci_base; the moriio paths are live-proven (Llama-70B TP, DeepSeek-V3 wideEP EP8,
0 failed requests). rixl+TP builds and NIXL initializes but hits an image-level
PyNCCL all-reduce error on this stack (cudagraph-independent) - tracked as a
known limitation, not a launcher regression.

Add a persistent, image-id-keyed JIT cache mount (host NVMe -> /opt/vllm_cache)
to both the production launcher and the interactive test driver. The image points
AITER_JIT_DIR/TRITON_CACHE_DIR/VLLM_CACHE_ROOT/COMGR_CACHE_DIR at /opt/vllm_cache,
which was previously unmounted, so AITER CK kernels recompiled (~15 min) every
run; now they compile once and warm-boot in ~1 min. Bump container shm to 256G.

Co-Authored-By: Claude <noreply@anthropic.com>
Drop the stale internal Slurm job number from run_interactive.sh's header;
describe its role (driven by tests/drive_cell.sh via srun --overlap) instead.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 3, 2026 10:06
@raviguptaamd

Copy link
Copy Markdown
Contributor Author

Copilot review — all findings addressed

Thanks Copilot. Each item has been fixed in the current head (c1c1339):

Finding Resolution
MODEL_PATH=$MODEL_PATH no-op / unset not guarded Now : "${MODEL_PATH:?MODEL_PATH must be set ...}" — fails fast with a clear message
ls ${NIXL_COOKBOOK_PATH} unquoted / runs when unset Guarded + quoted: [[ -n "${NIXL_COOKBOOK_PATH:-}" ]] && ls "${NIXL_COOKBOOK_PATH}"
WARN echo embeds MODEL_NAME unescaped (injection) The warn path is now pure Python (print(..., file=sys.stderr)); no shell eval of model-derived text
benchmark_parser.extract() returns float for integer counters Returns int(v) if v.is_integer() else v — counters/token totals stay integers
AITER post-install assert pinned to 0.1.14 Corrected to 0.1.16.post3 (both verification asserts)
DOCKER_IMAGE_NAME unset message only named the MoRI-EP Dockerfile Now names both Dockerfiles (rixl/NIXL vs MoRI-EP)
Header said default CONNECTOR=moriio Header now states the actual default (rixl via back-compat shim)
README claimed moriio uses a fixed toy proxy README now documents PROXY_TYPE=vllm_router (default) and the per-connector toy proxies
models.yaml header contradicted the tp: entries carrying --tensor-parallel-size Header now explains the tp: blocks intentionally carry the TP degree (legacy-map parity) and how moriio+TP de-dupes it
rixl TP env exported unconditionally vs "yaml can override" Connector values are defaults; the driver exports models.yaml env: afterward and a submit-time -e wins (precedence documented)

Additionally, since the review the image was consolidated: the combined all-connector image (moriio + rixl/NIXL + DeepEP) now builds from the single docker/vllm_disagg_inference.ubuntu.amd.Dockerfile (WITH_NIXL=1 default), and a persistent image-keyed JIT cache mount was added to avoid recompiling AITER kernels every run.

Note: rixl+TP currently hits an image/stack-level PyNCCL all-reduce error (unhandled cuda error, independent of cudagraph mode) that is not a launcher issue; the moriio TP and MoRI-EP wideEP paths (incl. full DeepSeek-V3) are live-validated with 0 failed requests.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Comment on lines +219 to +222
--all2all-backend "${_all2all}" \
--trust-remote-code \
--distributed-timeout-seconds "${DISTRIBUTED_TIMEOUT_SECONDS:-7200}" \
"${exec_args[@]}" "${extra_args[@]}" "${kv_args[@]}"
Comment on lines +242 to +245
"${exec_args[@]}" \
"${extra_args[@]}" \
"${kv_args[@]}" \
2>&1 | tee /run_logs/${SLURM_JOB_ID}/${log_prefix}_NODE${NODE_RANK}.log >/dev/null &
raviguptaamd and others added 2 commits July 3, 2026 10:14
…tifact)

The Docker image now ships only build + structural ENV (compile arch, source
pins, lib paths, and the /opt/vllm_cache cache locations). Everything run-tunable
is applied at launch, so the same image serves any model/cluster without a rebuild:

  - model-serving recipe (KV_BLOCK_SIZE, KV_CACHE_DTYPE, per-role *_CUDAGRAPH_MODE,
    *_MORI_BACKEND, GPU_MEMORY_UTILIZATION, KV_CACHE_MEMORY_BYTES, AITER_MLA, ...)
    -> scripts/vllm_dissag/models.yaml per-model env: (single source of truth).
    Added the per-role cudagraph defaults (prefill NONE, decode PIECEWISE) to the
    dense model entries, which previously inherited them from the image-level ENV.
  - MoRI/RDMA fabric tuning (MORI_RDMA_TC/SL, MORI_IB_GID_INDEX, MORI_NUM_QP_PER_PE,
    VLLM_MORIIO_*, HSA_*) -> connectors/moriio.env, alongside the platform env. These
    are cluster-specific (OCI MI300X RoCEv2 values), so baking them into a public
    image was wrong; overriding per-fabric now needs no rebuild.

Baking recipe defaults globally also leaked DeepSeek-specific values (AITER_MLA=0,
20GB KV) onto dense models; per-model env: fixes that. Launcher ${VAR:-default}
fallbacks match the previously-baked values, so the validated moriio TP and MoRI-EP
paths are unchanged (verified: dense decode still emits cudagraph PIECEWISE from
models.yaml; offline gate/argv/parity suites all pass).

Co-Authored-By: Claude <noreply@anthropic.com>
The combined vllm_disagg_inference Dockerfile is a superset: WITH_NIXL=0 already
produces exactly what vllm_disagg_mori_ep_fullsource did (MoRI-EP only: moriio
TP/wideEP + deepep-from-base), and WITH_NIXL=1 (default) adds the rixl NIXL/DeepEP
transport. Remove the duplicate fullsource Dockerfile and update the README + slurm
error message + in-file build hint to reference the single Dockerfile.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 3, 2026 10:29

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Comment on lines +218 to +222
--no-enable-prefix-caching \
--all2all-backend "${_all2all}" \
--trust-remote-code \
--distributed-timeout-seconds "${DISTRIBUTED_TIMEOUT_SECONDS:-7200}" \
"${exec_args[@]}" "${extra_args[@]}" "${kv_args[@]}"
Comment on lines +242 to +245
"${exec_args[@]}" \
"${extra_args[@]}" \
"${kv_args[@]}" \
2>&1 | tee /run_logs/${SLURM_JOB_ID}/${log_prefix}_NODE${NODE_RANK}.log >/dev/null &
| MoRI EP | 20005 | 10001 (fixed, MoRI-specific) |
| Connector | vLLM server port | Proxy port |
|-----------|------------------|------------|
| `rixl` (TP or DeepEP) | 2584 | `ROUTER_PORT` (30000 default) |
raviguptaamd added a commit to raviguptaamd/MAD that referenced this pull request Jul 3, 2026
Ports the GLM-5.1-FP8 WideEP enablement (from MAD-private #338) onto the
unified two-axis launcher (ROCm#171). GLM-5.1 is GlmMoeDsaForCausalLM: MLA +
DeepSeek Sparse Attention, served under MoRI-EP wideEP disagg at 1P1D (EP=8)
and 2P2D (EP=16).

Changes:
- run_xPyD_models.slurm: GLM-5.1-FP8 added to VALID_MODELS,
  MORI_EP_VALID_MODELS, and WIDE_EP_ONLY_MODELS (DSA validated only under
  wideEP; reject the untested moriio+TP path).
- models.yaml: GLM-5.1-FP8 entry. env: encodes the DSA recipe (KV_BLOCK_SIZE=1,
  VLLM_ROCM_USE_AITER_MLA=1, eager both roles, per-role mori all2all, 16GiB
  shmem heap). dp_flags carries the GLM tool/reasoning parsers.
- connectors/moriio.sh:
  * wideEP serve now appends model_args (per-model models.yaml flags) in both
    the dry-run and real paths. Previously only the TP path did, so wideEP
    dp:/dp_flags were silently dropped (rixl already appended them in both
    paths). No-op for models with empty dp flags (DeepSeek).
  * connector_runtime_patch() applies the 4 GLM DSA patchers, gated on
    MODEL_NAME=GLM-5.1-FP8; a pure no-op for every other model.
- 4 required DSA patchers (+1 optional instrument), idempotent + anchor-based
  + self-skipping: kernel invalid-token fix (vllm #45324), MoRIIO connector
  dual-KV geometry, engine per-layer transfer-offset cache, and the completion
  gate fix (gate on num_transfer_layers, excluding the DSA indexer caches).

DeepSeek/other models are byte-identical in behavior. Verified: bash -n,
py_compile, yaml lint, and DRY_RUN driver argv for GLM 1P1D/2P2D + DeepSeek
no-regression.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants