Skip to content

Commit beaed21

Browse files
committed
Merge upstream/develop into aicomnet_dev_slurm_multi
Merges 4 upstream commits (ef87d05..3a68be8) into the multi-node SLURM branch: - feat: canonicalize launcher aliases; extract MAD_MULTI_NODE_RUNNER resolver into _resolve_local_multi_node_runner_env() (upstream ROCm#132) - fix(run): --skip-model-run now starts container before skipping model script (ROCm#145) - chore: consolidate optional deps into default dependencies (ROCm#146) - Guyen/develop build-context folder change 2 (ROCm#144) Conflict in src/madengine/execution/container_runner.py resolved: - Upstream's if/else skip_model_run wrapper is the outer structure - Our try/except RuntimeError (container diagnostics on failure) is placed inside the else: branch around model_docker.sh() - Our elif skip_perf_collection: status branch and fallback or-clause are preserved in the else: branch status logic
1 parent a6b88b2 commit beaed21

22 files changed

Lines changed: 1037 additions & 543 deletions

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,4 @@ rocm_trace_lite_output/
144144
slurm_results/
145145
MagicMock/
146146
.madengine_session_start
147-
run_directory/
147+
run_directory/

CHANGELOG.md

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
- **`pytest` lower bound pinned to `>=7.0`**: Aligns the dependency pin with `minversion = "7.0"` already declared in `[tool.pytest.ini_options]`, preventing accidental resolution of older pytest versions that cannot run this project's tests.
1717

18+
### Changed
19+
20+
- **`--skip-model-run` now matches v1 semantics**: The flag previously short-circuited the entire run before any container started, and only took effect when a build ran in the same invocation (otherwise it was ignored with a warning). It now starts the container and runs `pre_scripts` and `post_scripts` as normal, skipping **only** the model script invocation — regardless of whether a build ran. The skip decision was moved out of `RunOrchestrator` and into `ContainerRunner`, so it applies uniformly to build+run and manifest-only (`--manifest-file`) invocations.
21+
22+
- **`--skip-model-run` runs report `SKIPPED`, not `FAILURE`**: A skipped model is now aggregated as a successful run with status `SKIPPED`, and the overall workflow exits `0`. Previously a skipped run could surface as a failure.
23+
24+
### Added
25+
26+
- **`--skip-model-run --keep-alive` for live container debugging**: Combining the two flags leaves a fully-set-up container alive after the skipped run, ready for manual exec (`docker exec -it <container> bash`). When `--keep-alive` is set, the run prints the exact `cd <model_dir> && <script> <args>` command to invoke the model by hand; otherwise it hints to re-run with `--keep-alive`.
27+
28+
- **Warning for local-only flags on distributed targets**: Passing `--skip-model-run`, `--keep-alive`, or `--keep-model-dir` with a SLURM or Kubernetes target now prints a yellow warning that these local Docker-only flags are ignored.
29+
1830
### Fixed
1931

20-
- **`tools/` build context path corrected**: `docker build` now resolves the shared tools directory as `./tools` (project root) instead of `./scripts/common/tools`. The previous path was stale — `scripts/common/tools` is a temporary directory populated at runtime by `madengine run`, so it was absent during standalone `madengine build` invocations, silently omitting the `--build-context tools=…` flag and breaking Dockerfiles that rely on it via `COPY --from=tools`.
32+
- **`tools/` build context path corrected**: `docker build` now resolves the shared tools directory as `./docker/common` (project root) instead of `./scripts/common/tools`. The previous path was stale — `scripts/common/tools` is a temporary directory populated at runtime by `madengine run`, so it was absent during standalone `madengine build` invocations, silently omitting the `--build-context tools=…` flag and breaking Dockerfiles that rely on it via `COPY --from=tools`.
2133

2234
- **Hatch package artifacts include `scripts/`**: `pyproject.toml` now uses `[tool.hatch.build.artifacts]` to include the `scripts/` directory in the built wheel. The previous `force-include` directive caused `duplicate file` errors with newer hatchling versions (which are stricter about files already covered by the default source inclusion). Switching to `artifacts` bypasses `.gitignore` exclusion without risk of duplication. The `deployment/templates` force-include was also removed as it is already captured by the default wheel source scan.
2335

@@ -621,8 +633,8 @@ black src/ tests/ && isort src/ tests/
621633
# Run pre-commit hooks manually
622634
pre-commit run --all-files
623635

624-
# Build without running (CI/CD)
625-
madengine run --tags model --skip-model-run
636+
# Skip model script (container starts, pre_scripts run); leave live container for debugging
637+
madengine run --tags model --skip-model-run --keep-alive
626638

627639
# Debug with verbose output
628640
madengine run --tags model --verbose --live-output

CLAUDE.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,72 @@
22

33
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
44

5+
## Behavioral Guidelines
6+
7+
Behavioral guidelines to reduce common LLM coding mistakes. Merge with project-specific instructions as needed.
8+
9+
**Tradeoff:** These guidelines bias toward caution over speed. For trivial tasks, use judgment.
10+
11+
## 1. Think Before Coding
12+
13+
**Don't assume. Don't hide confusion. Surface tradeoffs.**
14+
15+
Before implementing:
16+
- State your assumptions explicitly. If uncertain, ask.
17+
- If multiple interpretations exist, present them - don't pick silently.
18+
- If a simpler approach exists, say so. Push back when warranted.
19+
- If something is unclear, stop. Name what's confusing. Ask.
20+
21+
## 2. Simplicity First
22+
23+
**Minimum code that solves the problem. Nothing speculative.**
24+
25+
- No features beyond what was asked.
26+
- No abstractions for single-use code.
27+
- No "flexibility" or "configurability" that wasn't requested.
28+
- No error handling for impossible scenarios.
29+
- If you write 200 lines and it could be 50, rewrite it.
30+
31+
Ask yourself: "Would a senior engineer say this is overcomplicated?" If yes, simplify.
32+
33+
## 3. Surgical Changes
34+
35+
**Touch only what you must. Clean up only your own mess.**
36+
37+
When editing existing code:
38+
- Don't "improve" adjacent code, comments, or formatting.
39+
- Don't refactor things that aren't broken.
40+
- Match existing style, even if you'd do it differently.
41+
- If you notice unrelated dead code, mention it - don't delete it.
42+
43+
When your changes create orphans:
44+
- Remove imports/variables/functions that YOUR changes made unused.
45+
- Don't remove pre-existing dead code unless asked.
46+
47+
The test: Every changed line should trace directly to the user's request.
48+
49+
## 4. Goal-Driven Execution
50+
51+
**Define success criteria. Loop until verified.**
52+
53+
Transform tasks into verifiable goals:
54+
- "Add validation" → "Write tests for invalid inputs, then make them pass"
55+
- "Fix the bug" → "Write a test that reproduces it, then make it pass"
56+
- "Refactor X" → "Ensure tests pass before and after"
57+
58+
For multi-step tasks, state a brief plan:
59+
```
60+
1. [Step] → verify: [check]
61+
2. [Step] → verify: [check]
62+
3. [Step] → verify: [check]
63+
```
64+
65+
Strong success criteria let you loop independently. Weak criteria ("make it work") require constant clarification.
66+
67+
---
68+
69+
**These guidelines are working if:** fewer unnecessary changes in diffs, fewer rewrites due to overcomplication, and clarifying questions come before implementation rather than after mistakes.
70+
571
## Development Setup
672

773
```bash

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ After a local Docker run, madengine can scan the captured **run log** for common
587587
### Build & Deployment
588588

589589
- **Separate build and run phases** for distributed deployments
590-
- **Build without executing:** `madengine run --tags … --skip-model-run` skips container execution **after a build in that same invocation** (ignored when using an existing `--manifest-file`). See [Usage — Skip model run after build](docs/usage.md#skip-model-run-after-build).
590+
- **Skip model script:** `madengine run --tags … --skip-model-run` starts the container and runs `pre_scripts`, but skips the model script. Combine with `--keep-alive` for a live container ready for manual exec. Ignored with a warning on SLURM/K8s. See [Usage — Skip model run after build](docs/usage.md#skip-model-run-after-build).
591591
- **Use registries** for multi-node execution (K8s/SLURM)
592592
- **Use batch build mode** for CI/CD to optimize build times
593593
- **Specify `--target-archs`** when building for multiple GPU architectures

docs/cli-reference.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ madengine build [OPTIONS]
9797
| `--tags` | `-t` | TEXT | `[]` | Model tags to build (can specify multiple) |
9898
| `--target-archs` | `-a` | TEXT | `[]` | Target GPU architectures (e.g., gfx908,gfx90a,gfx942) |
9999
| `--registry` | `-r` | TEXT | `None` | Docker registry to push images to |
100-
| `--use-image` | | TEXT | `None` | Skip Docker build and use a pre-built image. Omit value or pass `auto` to resolve from model card's `DOCKER_IMAGE_NAME`. Mutually exclusive with `--registry` and `--build-on-compute` |
100+
| `--use-image` | | TEXT | `None` | Skip Docker build and use a pre-built image. Pass an image name, or `auto` to resolve from model card's `DOCKER_IMAGE_NAME`. Mutually exclusive with `--registry` and `--build-on-compute` |
101101
| `--build-on-compute` | | FLAG | `False` | Build Docker images on a SLURM compute node and push to registry. Requires `--registry` |
102102
| `--batch-manifest` | | TEXT | `None` | Input batch.json file for batch build mode |
103103
| `--additional-context` | `-c` | TEXT | `"{}"` | Additional context as JSON string |
@@ -149,7 +149,7 @@ madengine build --tags model --live-output --verbose
149149
madengine build --tags model --use-image lmsysorg/sglang:v0.5.2rc1-rocm700-mi30x
150150

151151
# Auto-detect image from model card's DOCKER_IMAGE_NAME
152-
madengine build --tags model --use-image
152+
madengine build --tags model --use-image auto
153153

154154
# Build on SLURM compute node and push to registry
155155
madengine build --tags model --build-on-compute --registry docker.io/myorg
@@ -226,10 +226,10 @@ madengine run [OPTIONS]
226226
| `--timeout` | | INT | `-1` | Timeout in seconds (-1=default 7200s, 0=no timeout) |
227227
| `--additional-context` | `-c` | TEXT | `"{}"` | Additional context as JSON string |
228228
| `--additional-context-file` | `-f` | TEXT | `None` | File containing additional context JSON |
229-
| `--keep-alive` | | FLAG | `False` | Keep Docker containers alive after run |
230-
| `--keep-model-dir` | | FLAG | `False` | Keep model directory after run |
229+
| `--keep-alive` | | FLAG | `False` | Keep Docker containers alive after run (local Docker only; ignored with a warning on SLURM/K8s) |
230+
| `--keep-model-dir` | | FLAG | `False` | Keep model directory after run (local Docker only; ignored with a warning on SLURM/K8s) |
231231
| `--clean-docker-cache` | | FLAG | `False` | Rebuild images without using cache (full workflow) |
232-
| `--skip-model-run` | | FLAG | `False` | After a **build in this invocation**, skip executing models (manifest/images still produced). **Ignored** when using `--manifest-file` with an existing manifest (run-only), or when no build ran in this invocation. See [Usage — Skip model run](usage.md#skip-model-run-after-build). |
232+
| `--skip-model-run` | | FLAG | `False` | Skip the model script inside each container. The container still starts and `pre_scripts` still run; only the model script invocation is skipped (status reported as `SKIPPED`, exit code `0`). Combine with `--keep-alive` to leave a live container for manual exec. Ignored with a warning on SLURM/K8s targets. See [Usage — Skip model run](usage.md#skip-model-run-after-build). |
233233
| `--manifest-output` | | TEXT | `build_manifest.json` | Output file for build manifest (full workflow) |
234234
| `--summary-output` | `-s` | TEXT | `None` | Output file for summary JSON |
235235
| `--live-output` | `-l` | FLAG | `False` | Print output in real-time |

docs/deployment.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ For workloads that use externally maintained Docker images (e.g. SGLang, vLLM re
303303
madengine build --tags model --use-image lmsysorg/sglang:latest
304304

305305
# Auto-detect image from model card's DOCKER_IMAGE_NAME
306-
madengine build --tags model --use-image
306+
madengine build --tags model --use-image auto
307307

308308
# Build on a SLURM compute node and push to registry
309309
madengine build --tags model --build-on-compute --registry docker.io/myorg

docs/launchers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ slurm_multi models typically use pre-built images. The build phase has a **regis
629629
madengine build --tags my_model --use-image lmsysorg/sglang:latest
630630

631631
# Auto-detect image from model card's DOCKER_IMAGE_NAME
632-
madengine build --tags my_model --use-image
632+
madengine build --tags my_model --use-image auto
633633

634634
# Build on compute node and push to registry
635635
madengine build --tags my_model --build-on-compute --registry docker.io/myorg

docs/usage.md

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,15 +283,25 @@ madengine build --batch-manifest batch.json \
283283

284284
### Skip model run after build
285285

286-
When `madengine run` **builds** in the same invocation (no pre-existing `--manifest-file`), you can pass **`--skip-model-run`** to produce images and `build_manifest.json` **without** running model containers.
286+
Pass **`--skip-model-run`** to start containers and run `pre_scripts`, but skip executing the model script itself.
287287

288-
- **Ignored** when `--manifest-file` points at an existing manifest (execution-only mode): use plain `madengine run --manifest-file ...` to run later.
289-
- **Ignored** with a warning if this invocation did not perform a build (for example a manifest was already present and no rebuild occurred).
288+
- The Docker container **is started** and `pre_scripts` run normally.
289+
- Only the model script invocation is skipped; `post_scripts` and container cleanup still run.
290+
- Exit status is `SKIPPED` (not `FAILURE`) — the overall run exits `0`.
291+
- Combine with **`--keep-alive`** to leave a fully-set-up, live container for manual inspection or debugging.
292+
- Has **no effect** on distributed (SLURM/K8s) targets — a warning is printed if used with them.
290293

291294
```bash
295+
# Skip the model script; container starts and pre_scripts run
292296
madengine run --tags model \
293297
--additional-context '{"gpu_vendor": "AMD", "guest_os": "UBUNTU"}' \
294298
--skip-model-run
299+
300+
# Leave a live container ready for manual exec
301+
madengine run --tags model \
302+
--additional-context '{"gpu_vendor": "AMD", "guest_os": "UBUNTU"}' \
303+
--skip-model-run --keep-alive
304+
# Then: docker exec -it <container> bash
295305
```
296306

297307
See [CLI Reference — `run`](cli-reference.md#run---execute-models) and `madengine run --help`.
@@ -425,9 +435,13 @@ madengine run --tags model --timeout 0
425435
### Debugging
426436

427437
```bash
428-
# Keep containers alive
438+
# Keep container alive after run (local Docker only)
429439
madengine run --tags model --keep-alive
430440

441+
# Skip model script and leave a live container ready for manual exec
442+
madengine run --tags model --skip-model-run --keep-alive
443+
# Then inspect: docker exec -it <container> bash
444+
431445
# Verbose output
432446
madengine run --tags model --verbose --live-output
433447

@@ -714,9 +728,12 @@ madengine run --tags model1 --tags model2,model3
714728
# Full verbose output with real-time logs
715729
madengine run --tags model --verbose --live-output
716730

717-
# Keep container alive for inspection
731+
# Keep container alive for inspection (local Docker only)
718732
madengine run --tags model --keep-alive
719733

734+
# Skip model script and leave live container for manual exec
735+
madengine run --tags model --skip-model-run --keep-alive
736+
720737
# Check what will be discovered
721738
madengine discover --tags model --verbose
722739
```

docs/wiki/index.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ <h2>Quick start</h2>
295295
madengine build --tags pyt_sglang_disagg --use-image registry.io/sglang:latest
296296

297297
# Option B — auto-resolve DOCKER_IMAGE_NAME from model card
298-
madengine build --tags pyt_sglang_disagg --use-image
298+
madengine build --tags pyt_sglang_disagg --use-image auto
299299

300300
# Option C — build on compute node, push, then run pulls in parallel
301301
madengine build --tags pyt_sglang_disagg \
@@ -621,7 +621,7 @@ <h4><code>--use-image</code> modes</h4>
621621
<table>
622622
<thead><tr><th>Invocation</th><th>Behavior</th></tr></thead>
623623
<tbody>
624-
<tr><td><code>--use-image</code> (bare flag)</td><td>Resolves to <code>"auto"</code> — reads <code>DOCKER_IMAGE_NAME</code> from model card <code>env_vars</code></td></tr>
624+
<tr><td><code>--use-image auto</code></td><td>Reads <code>DOCKER_IMAGE_NAME</code> from model card <code>env_vars</code></td></tr>
625625
<tr><td><code>--use-image registry.io/img:tag</code></td><td>Uses the explicit image name; skips all Docker build steps</td></tr>
626626
</tbody>
627627
</table>
@@ -1115,7 +1115,7 @@ <h3>Build modes</h3>
11151115
<thead><tr><th>Mode</th><th>Flag</th><th>Behavior</th></tr></thead>
11161116
<tbody>
11171117
<tr><td>Use prebuilt image</td><td><code>--use-image registry.io/img:tag</code></td><td>Skip local build. Uses explicit image.</td></tr>
1118-
<tr><td>Auto-resolve from model card</td><td><code>--use-image</code> (bare)</td><td>Reads <code>env_vars.DOCKER_IMAGE_NAME</code> from model card.</td></tr>
1118+
<tr><td>Auto-resolve from model card</td><td><code>--use-image auto</code></td><td>Reads <code>env_vars.DOCKER_IMAGE_NAME</code> from model card.</td></tr>
11191119
<tr><td>Build on compute</td><td><code>--build-on-compute --registry reg.io/ml</code></td><td>Builds on SLURM compute node, pushes to registry. Manifest sets <code>built_on_compute: true</code>. Run phase pulls in parallel on all nodes.</td></tr>
11201120
<tr><td>Implicit fallback</td><td>no flags</td><td>If model card has <code>DOCKER_IMAGE_NAME</code>, auto-uses it. Otherwise raises <code>ConfigurationError</code> listing options.</td></tr>
11211121
</tbody>

src/madengine/cli/commands/build.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,13 @@ def build(
5555
"--batch-manifest", help="Input batch.json file for batch build mode"
5656
),
5757
] = None,
58-
# NOTE: `is_flag=False, flag_value="auto"` lets `--use-image` (no value)
59-
# mean "auto-detect from the model card's DOCKER_IMAGE_NAME", matching
60-
# MAD-private PR #186's documented UX. Typer is deprecating this pattern
61-
# for a future release; when removed, switch to requiring an explicit
62-
# value (e.g. `--use-image auto` as the documented sentinel) and update
63-
# MAD-private's docs in lockstep.
58+
# Pass `--use-image auto` to auto-detect the image from the model card's
59+
# DOCKER_IMAGE_NAME, or `--use-image NAME` to use an explicit image.
6460
use_image: Annotated[
6561
Optional[str],
6662
typer.Option(
6763
"--use-image",
68-
is_flag=False,
69-
flag_value="auto",
70-
help="Skip Docker build and use pre-built image. Optionally specify image name, or omit to auto-detect from model card's DOCKER_IMAGE_NAME"
64+
help="Skip Docker build and use a pre-built image. Pass an image name, or 'auto' to auto-detect from the model card's DOCKER_IMAGE_NAME",
7165
),
7266
] = None,
7367
build_on_compute: Annotated[

0 commit comments

Comments
 (0)