Skip to content

Refuse launch shapes the partition can't run, and keep a matching catalog layout (#175, #112) - #184

Open
dadamsncsa wants to merge 26 commits into
mainfrom
request-validation
Open

dadamsncsa wants to merge 26 commits into
mainfrom
request-validation

Conversation

@dadamsncsa

Copy link
Copy Markdown
Contributor

Aimed at main so CI runs. It still contains #177's and #178's commits, and a merge of main
(4c47f40): review and merge #177 and #178 first. This PR's own commits are aaab055,
9a948f5, 264b2c7, ecc276e, b3bf5ff, 8936a51, 4bd93fa and 732ce94.

Closes #175, and with it #112. Part of #60.

What changes

A launch request that sets num_gpus or num_nodes gets one layout (backend/app/utils/launch_layout.py):

ModelService.launch_model then checks it before any download, launcher or row. These are answered 422 with the reason, and nothing is submitted or recorded:

Shape Reason starts
more nodes than the partition's MaxNodes (sinfo %s, min-max or one number) N nodes requested; partition P allows at most M per job
more GPUs per node than the partition's nodes have N GPUs per node requested (tensor parallel T); partition P has M per node
TP across nodes tensor parallel T would span nodes of G GPUs each
multinode SGLang multinode launches run on vLLM only
multinode on a vec-inf the #177 adapter wasn't tested against multinode launch refused: vec-inf V is not a version …
what vec-inf would refuse after the download: a count below 1, a GPU total not a power of two, TP x PP not the GPU total … must each be at least 1, … vec-inf launches a power of two, … does not match the N GPUs requested
fewer nodes than the partition's MinNodes N nodes requested; partition P needs at least M per job
a partition Slurm doesn't list, or a list naming none partition P is not a partition Slurm lists

The partition's limits come from sinfo --all (%P|%s|%G) when the request is checked. Of a comma-separated list, every partition must be one Slurm lists and must fit its node limits, since Slurm checks a job's partition limits against each partition listed (DeltaAI: EnforcePartLimits=ALL; sbatch --test-only rejects the whole list otherwise). One listed partition with the GPUs per node is enough. A refusal names every limit the request breaks. If sinfo fails, the partition checks are skipped (logged) and Slurm's own submit-time limits apply.

Unchanged: a request without counts, which is what the UI sends, runs the catalog's layout, unchecked, exactly as before. There is no new config and nothing changes in infra/delta.

Evidence (DeltaAI gh-login03, at 732ce94)

  • tests/test_launch_layout.py: 25 passed. Each refusal goes through the real POST /api/models/deployments, ModelService and vec-inf's own launch against a fake sbatch. It asserts 422, the reason, no vec-inf launcher built, no script, no download and no row. A control shows a runnable 2-node request is submitted.
  • Removal records: each check removed in turn from a git archive of the head, and its tests exit 1 (18 removals, plus the unedited control at exit 0):
  • UI launches unchanged: every entry of each cluster's rendered catalog, POSTed as the UI sends it, renders the same script before (4c47f40) and after (732ce94).
    • DeltaAI: 90 entries, 90 submitted, 0 differ.
    • Delta: 94 entries, 94 submitted, 0 differ.
    • A planted --max-model-len change to one entry is reported on each.
  • Full backend suite: 322 passed, 1 skipped. black, isort and flake8 are clean.
  • Reviews: /simplify, /code-review, a re-review, a fresh reader and two more /code-review rounds; each finding is fixed or answered. 4bd93fa (the fix for the last review's findings) and 732ce94 (CHANGELOG only) have not been reviewed by an agent.

Rollback

Revert the PR. It has no config or schema change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2

dadamsncsa and others added 26 commits November 6, 2026 12:21
vec-inf 0.9.0 launches num_nodes > 1 vLLM as a Ray cluster in N+1 srun
steps. On DeltaAI each step gets its own Slingshot VNI, so the ranks
cannot reach each other over CXI.

For multinode vLLM only, LLMHub now builds MultinodeLauncher, a vec-inf
ModelLauncher whose generator replaces the Ray branch. The job runs one
srun step with one task per node and --kill-on-bad-exit=1. A rank script,
written into the job directory, derives the node's hsn<digits> address,
waits on a barrier, and execs the same container vec-inf would. Rank 0
runs vllm serve with the API; the other ranks run it --headless. They
join through vLLM's --nnodes/--node-rank/--master-addr/--master-port
with the mp executor. vLLM runs under `env -u FI_LOG_LEVEL`, so the
image's libfabric warnings cannot make vec-inf report a serving job as
FAILED. Single-node launches, and anything that is not vLLM, go to
vec-inf unchanged. The route is chosen on vec-inf's resolved params, so
the catalog's node count counts.

The adapter refuses a multinode launch on any vec-inf other than 0.9.0.
Contract tests T1-T5 cover the version guard, single-node byte
identity over the catalog, the job's shape, the rank script run with
stubbed tools, and the vec-inf names it depends on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
Drop the heredoc-delimiter guard (no generated line can equal it), inline
the job-directory helper, and leave the multinode check to the generator,
which refuses before sbatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
- Clear the rendezvous directory before srun: a requeued job keeps its id,
  and stale rank files would satisfy the barrier at once.
- Decide the NCCL debug file on the rank, so NCCL_DEBUG reaching the
  container through APPTAINERENV_* is caught too, not only the job env.
- Time the barrier by the wall clock, not by loop iterations.
- Document that a non-container multinode launch is refused, and that the
  rank's --env flags rely on llm_inference's one-flag-per-variable patch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
… fixes

Behaviour change: every multinode launch now records NCCL INFO for INIT
and NET, written to one file per process in the job directory's
nccl-debug/ and never to stderr. That is how a multinode job shows which
transport each rank chose (provider, NIC, GDR). It applies unless the job
env or the host sets a level, and a NCCL_DEBUG_FILE given by either is
kept.

From the fresh reader:
- A rank whose srun step has no CUDA_VISIBLE_DEVICES fails with a reason,
  not a bash "unbound variable".
- A job env that sets VLLM_HOST_IP, NCCL_SOCKET_IFNAME or
  GLOO_SOCKET_IFNAME is refused: each rank derives them, and --env would
  silently win.
- The master-port probe's stderr stays out of the job's .err.
- T5 also names ModelLauncher.slurm_script_path, which the override sets
  and launch() reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…check

- The job env's VLLM_HOST_IP, NCCL_SOCKET_IFNAME, GLOO_SOCKET_IFNAME and
  VLLM_PORT are left out of a multinode job's container env and noted in
  the job log, not refused: each rank sets its own, and a deployment may
  set them for single-node jobs.
- With no NCCL level anywhere, the level is INFO even when a debug file is
  named.
- A rank without CUDA_VISIBLE_DEVICES fails before the barrier, so no other
  rank waits on it; a rank that cannot write its barrier marker says so.
- The master-port probe's stderr is visible again: a failing ss should
  leave a trace.
- T4 runs the rank script for each NCCL case, job env and host alike.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
- The job JSON records the env the job was given, without the per-rank
  keys left out of it.
- A missing ss is reported on stdout; ss's own stderr stays out of the
  .err that vec-inf scans for fatal patterns.
- A container module other than apptainer is refused: the rank script
  passes its values as APPTAINERENV_* and binds through APPTAINER_BINDPATH.
- T3 and T4 also cover a job env that sets both the NCCL level and file;
  T5 names CONTAINER_MODULE_NAME.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…ted in full

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…odes (#174)

- LLMHUB_VEC_INF_ENV carries FI_CXI_RDZV_THRESHOLD=8192 and
  FI_CXI_RDZV_GET_MIN=0 on both clusters until the images bake them
  (GAP-098: cross-node all_reduce corruption on DeltaAI, job 3533388).
- DeltaAI's job pre-command exports SLURM_NETWORK=disable_rdzv_get, the
  site value; a multi-node step gets one VNI under either value.
- DeltaAI renders its catalog through config/models-deltaai.yaml
  (render-models.py, LLMHUB_MODELS_OVERRIDE): five 2-node A100 entries at
  1 node TP=4, no Mllama, no 405B, and Qwen3-235B at 2 nodes TP=4 PP=2
  with 400G per node. Delta renders backend/config/models.yaml as is.
- launch-test --nodes N. At N > 1 it checks one rank per node, every
  worker and stage loaded, NCCL on CXI, the pair in every rank's
  container, no libfabric line before ready, and that DELETE leaves the
  job CANCELLED with no node held.
- T2 now compares each cluster's kit-rendered catalog, job env and
  pre-command. New tests check the kit's values and load DeltaAI's catalog
  with vec-inf's load_config().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…cct retry

The multinode checks read an impersonated job's files directly, as the
backend's GET /deployments/{id}/logs does, through the workspace's default
ACL for the service account, instead of an untried `sudo -u`. The
post-DELETE state check waits up to 60 s for sacct to leave
RUNNING/PENDING.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…aunch-test checks

From /code-review and the fresh reader on #178:
- vec-inf merges a cluster's models.yaml over a base catalog, its packaged
  one by default, so the entries DeltaAI's override removes (405B, Mllama)
  came back at launch. With an override set, the rendered environment.yaml
  now names the rendered models.yaml as that base (cached_model_config_path).
  The environment render moves from inline Python to render-vec-inf-env.py,
  which the tests now call, so T2 and the kit tests use the kit's own render.
- render-models.py: a `models:` name the base lacks is an error; new
  entries go under `add:`.
- The five 2-node entries moved onto one GH200 node get 400G per node.
  None of them has been run on GH200 yet.
- launch-test --nodes N:
  - --nodes is validated before use;
  - Slurm reads after "ready" cannot abort the script before DELETE (an
    impersonated job is the backend's to cancel);
  - a kit-side scancel fails the DELETE check;
  - "nodes freed" now checks the nodes' states in sinfo;
  - stages are counted by Worker_PP prefix;
  - CXI is counted per worker file;
  - GDRDMA is required on DeltaAI;
  - the pair is matched as whole fields;
  - the libfabric count needs the ready line.
- Tests: the launch-path catalog (load_config() with no path, as vec-inf
  calls it) and the override typo errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…against the live server

The client runs after launch-test's own inference checks and before its
DELETE. It gets the endpoint, the deployment key, the served name and the
job id in its environment, never argv, and its exit status counts toward
launch-test's. This is how the Qwen3-235B acceptance run goes through
LLMHub's launch and shutdown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…DELETE timestamps; no Mllama on DeltaAI

- launch-test's ready budget (the job's walltime) is counted only while
  Slurm runs the job, so a long queue cannot use it up before a large
  model has loaded. Pending has its own 4 h cap.
- The shutdown records the DELETE time, sacct's Start and End, and the
  node states with a timestamp: the acceptance check is "free within 60 s".
- DeltaAI leaves out all four Mllama entries. The two 11B entries join the
  90B ones: neither image registers Mllama, and 11B-Vision-Instruct had
  resolved to SGLang through vec-inf's packaged catalog, whose image is
  not on DeltaAI. DeltaAI renders 90 entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7f83LXZjJVqJ3RLAAD
…atching catalog layout (#175, #112)

A request that sets num_gpus or num_nodes now gets one layout function
(app/utils/launch_layout.py): the catalog entry's own TP/PP flags when its
GPUs per node and node count match the entry's, otherwise TP = GPUs per node
and PP = nodes (#112). Before, any explicit count replaced the catalog's
layout.

That request is checked in ModelService.launch_model, before any download,
launcher or row, against the partition it will run on as sinfo reports it.
These are answered 422 with the reason: more nodes than the partition's
MaxNodes, more GPUs per node than its nodes have, tensor parallel across
nodes, multinode SGLang, and multinode on a vec-inf the multinode adapter
wasn't tested against. A request without counts (the UI's) runs the
catalog's layout unchecked, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
resolve_layout takes the request params; the three multinode refusals share
one block; MaxNodes is read once (sinfo's %s is partition-wide); _size drops a
branch int() already covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…fuse an unlisted partition

From /code-review on PR 3. A request with a GPU total that isn't a power of
two, a count below 1, or a catalog layout whose TP x PP isn't the GPU total
passed the check, started its download, and failed in vec-inf with a failed
row. They are now refused at the API like the other shapes. A partition sinfo
answers for with no lines (a working sinfo, no such partition) is refused; a
comma-separated partition list allows the most of each; sinfo's timeout is
10 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…ne partition that fits

From PR 3's re-review. A comma-separated partition list took MaxNodes from one
partition and GPUs per node from another, so it could pass a shape no listed
partition runs; it now passes only if one partition fits both. sinfo runs once
with --all (hidden partitions, and ones closed to the backend's account,
count: the job is submitted as another user). A list naming no partition is
refused, and the fake sinfo in the tests answers DeltaAI's whole table, so a
wrong partition name fails a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…nNodes; every reason

From the extra /code-review of ecc276e. DeltaAI runs EnforcePartLimits=ALL, so
sbatch rejects a partition list unless the job fits every partition listed
(checked with sbatch --test-only): a list now passes only if every name is a
partition and fits, not any one. A request below a partition's MinNodes is
refused too, and a refusal names every limit the request breaks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2
…ted partition

From the /code-review of ecc276e..8936a51. sinfo prints %s as one number when
MinNodes equals MaxNodes ("1", not "1-1"); that number is now both limits
(b3bf5ff had read it as MinNodes only, an unlimited MaxNodes). Rows of one
partition combine to the tightest node limits. EnforcePartLimits holds a job
to every listed partition's node limits, but GPUs per node only need one
listed partition that has them. Names in a list are de-duplicated, and the
docstrings list every partition check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7fZZkKzxYacTgvKLL2

This branch has not been deployed

No deployments
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.

Validate a launch's parallel layout before submitting, and refuse shapes the cluster can't run

1 participant