Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6741674
fix(slurm): honor the model-card contract on the slurm_multi path
Cemberk Aug 17, 2026
def98bf
fix(run): a self-managed SLURM launcher implies a SLURM deployment
Cemberk Aug 17, 2026
89b3fcd
Merge branch 'develop' into cem/slurm-two-path-parity
coketaste Aug 21, 2026
3344a2d
Merge branch 'develop' into cem/slurm-two-path-parity
coketaste Sep 23, 2026
b18b264
fix(slurm): reject non-positive distributed.nnodes; drop a tautologic…
coketaste Sep 23, 2026
bc79022
fix(slurm): persist launcher/slurm.* for all build paths, track expli…
coketaste Sep 23, 2026
5f67ee7
fix(slurm): close the three open Copilot findings on the parity path
coketaste Sep 23, 2026
390d795
fix(slurm): persist empty explicit_slurm_keys provenance for manifest…
coketaste Sep 24, 2026
8df5101
fix(slurm): reject non-integral/non-finite distributed.nnodes
coketaste Sep 24, 2026
025496b
Merge branch 'develop' into cem/slurm-two-path-parity
coketaste Oct 1, 2026
f691bd1
test: set docker_run_cmd on mocked Docker in container runner tests
coketaste Oct 1, 2026
29ce990
fix(build): treat whitespace-only DOCKER_IMAGE_NAME as a placeholder
coketaste Oct 1, 2026
48d69a5
fix(slurm): ignore stale declared multiple_results CSV from earlier runs
coketaste Oct 1, 2026
9595b2d
fix(build): merge deployment metadata only from successfully built mo…
coketaste Oct 1, 2026
7856614
fix(run): infer slurm target from the model card when runtime drops t…
coketaste Oct 1, 2026
50e47ea
test(run): cover stored local target not demoting a slurm_multi card
coketaste Oct 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/launchers.md
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,44 @@ madengine run --manifest-file build_manifest.json

**Alias**: `"slurm-multi"` (hyphen) is normalized to `"slurm_multi"` (underscore).

**Model-card contract (shared with the templated path)**:

slurm_multi hands the *topology* to the workload, not the *contract*. These model-card
fields are honoured identically on both paths:

| Field | Effect |
|-------|--------|
| `distributed.launcher` | Selects the path. Resolved deployment-config-first, model-card-second — the same resolution used to emit the launcher env block, so the two can never disagree. |
| `distributed.nnodes` | Sizes the allocation (`#SBATCH --nodes` / `--ntasks`) when `slurm.nodes` is not set explicitly. If both are set and differ, `slurm.nodes` wins and a warning is printed. Non-numeric or non-positive values are ignored with a warning — `slurm.nodes` is used instead. |
| `slurm.*` | `partition`, `nodes`, `gpus_per_node`, `time`, `exclusive`, `reservation`, `output_dir`, `nodelist`, `account`, `qos`, `modules`, `skip_gpus_directive`, `results_dir` — the full set every build path (normal build, prebuilt image, `--build-on-compute`) promotes into `deployment_config.slurm`. Other `slurm.*` fields documented in [configuration.md](configuration.md) are read at deploy time but are not copied from the model card into a manifest's `deployment_config`. |
| `multiple_results` | Names the results CSV. Searched next to the model script (where the wrapper `cd`s, so `$(pwd)` writes land there), then the job dir, output dir and cwd, before falling back to the conventional locations. |

> **`args` is not sbatch flags.** On the templated path `args` goes to the
> containerised `run.sh`; on this path it is appended to `bash <model>.slurm`. It is
> **not** forwarded to `sbatch`, so `"args": "-N 4 -n 4"` does *not* request 4 nodes —
> it passes two ignored positional arguments to a script that never reads `$@`, and
> the job runs on the `slurm.nodes` default of 1. Size the allocation with
> `distributed.nnodes` (or `slurm.nodes`) instead.

> **`multiple_results` is read directly here.** On the templated path the CSV is
> narrow and gets merged with `common_info` by `update_perf_csv`. A self-managed
> script has no `common_info` to merge against, so it writes the full perf schema
> itself and madengine reads it as-is — routing it through `handle_multiple_results()`
> would recompute `status` from `performance` and flip a legitimate zero-score
> FAILURE row to SUCCESS.

> **Preset selection still keys on `slurm.nodes`.** `ConfigLoader` picks the
> single-node vs multi-node preset before `nnodes` reconciliation, so a card sized
> only by `nnodes` keeps the single-node preset. That is usually what RDMA inference
> workloads want — the multi-node preset sets `NCCL_IB_DISABLE=1` and
> `NCCL_SOCKET_IFNAME=eth0` — but set `slurm.time` explicitly if the 12 h single-node
> default is too short.

**Placeholder images are rejected**: model cards commonly ship
`"DOCKER_IMAGE_NAME": "<supply-your-image>"` as a fill-me-in marker. Any
angle-bracketed value is rejected at submit time with an actionable error rather than
becoming the image every compute node fails to pull.

**Features**:
- Wrapper SBATCH script with shell-quoted env_vars (injection-safe)
- Parallel `srun docker pull` on all nodes for registry images
Expand Down
82 changes: 82 additions & 0 deletions src/madengine/deployment/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,88 @@ def is_self_managed_launcher(launcher_type: Optional[str]) -> bool:
return normalize_launcher(launcher_type, "slurm") in _SELF_MANAGED_LAUNCHERS


def resolve_launcher_from_sources(
deployment_launcher: Optional[str],
model_launcher: Optional[str],
default: str = "torchrun",
) -> str:
"""Resolve the effective launcher from the deployment config and the model card.

Precedence is deployment config first, model card second. That matches how
BuildOrchestrator builds the manifest: it copies the model card's launcher into
``deployment_config.distributed`` only when the key is absent, so a value present
there is already the user's explicit choice.

Every dispatch site must use this helper. Resolving the launcher differently in
two places is what allowed ``prepare()`` to pick the self-managed path from the
model card while ``_prepare_template_context()`` simultaneously emitted a
``torchrun`` env block from the deployment config.
"""
return deployment_launcher or model_launcher or default


def resolve_node_count(
configured_nodes: int,
nnodes: Optional[Any],
nodes_explicitly_set: bool,
) -> tuple:
"""Reconcile ``slurm.nodes`` with ``distributed.nnodes`` into one node count.

``slurm.nodes`` sizes the allocation (``#SBATCH --nodes``) while
``distributed.nnodes`` sizes the topology the launcher builds on top of it.
Nothing used to reconcile them, so a model card declaring only ``nnodes`` was
submitted against the ``slurm.nodes`` default of 1 — a multi-node topology
squeezed onto a single node.

Args:
configured_nodes: ``slurm.nodes`` after ConfigLoader applied its defaults.
nnodes: ``distributed.nnodes``, if declared.
nodes_explicitly_set: True when ``slurm.nodes`` came from the user or the
model card rather than from a preset default.

Returns:
``(nodes, note)`` where ``note`` is a human-readable string to log, or None.
"""
try:
if isinstance(nnodes, bool) or (
isinstance(nnodes, float) and not nnodes.is_integer()
):
raise ValueError
requested = int(nnodes) if nnodes is not None else None
except (TypeError, ValueError, OverflowError):
return configured_nodes, (
f"Ignoring non-numeric distributed.nnodes={nnodes!r}; "
f"using slurm.nodes={configured_nodes}."
)

if requested is not None and requested < 1:
# An allocation of zero or fewer nodes is not a thing sbatch can schedule;
# passing it through would fail at submit time with a scheduler error that
# says nothing about the model card that caused it.
return configured_nodes, (
f"Ignoring distributed.nnodes={requested} (must be >= 1); "
f"using slurm.nodes={configured_nodes}."
)

if requested is None or requested == configured_nodes:
return configured_nodes, None

if nodes_explicitly_set:
# Both were set and they disagree. The explicit allocation size wins —
# overriding it could request nodes the user did not ask to be billed for —
# but the mismatch is almost always a config error, so say so.
return configured_nodes, (
f"slurm.nodes={configured_nodes} conflicts with "
f"distributed.nnodes={requested}; using slurm.nodes={configured_nodes}. "
"Set them to the same value to silence this."
)

return requested, (
f"Sizing allocation from distributed.nnodes={requested} "
f"(slurm.nodes was not set explicitly)."
)


@functools.lru_cache(maxsize=None)
def is_rocprofv3_available() -> bool:
"""
Expand Down
Loading
Loading