Skip to content

Commit 0c89b9e

Browse files
Ronald Tseronaldtse
authored andcommitted
refactor: round 2 — runstate markers, resolve_spec, versioned supervision (R1/R2/R4)
R1 runstate: the marker protocol (best/config.json, final_eval.json, chain_log.jsonl, step-N) gets an interface — RunState, mkdir-safe by construction (the arm-4 bug class is unrepresentable). The orchestrator consumes it; pure-CPU tests. R2 supervision as code: scripts/modal_retry.sh + scripts/watch_marker.sh replace the seven bespoke /tmp loops this campaign produced. R4 resolve_spec: the four pasted vol_map blocks collapse into one owner of 'which volume does this spec's teacher/student/dataset live on', verified against real specs (rababa/secrets/persian mounts, hub teachers, data_volume overrides).
1 parent 05124e1 commit 0c89b9e

5 files changed

Lines changed: 199 additions & 80 deletions

File tree

scripts/modal_retry.sh

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/bin/bash
2+
# Generic Modal retry wrapper — the resilience pattern this campaign
3+
# re-implemented in /tmp seven times. Idempotent server-side functions
4+
# make retries the resume mechanism.
5+
#
6+
# scripts/modal_retry.sh <file::function> [args...] [retry_sleep_secs]
7+
#
8+
# Example:
9+
# scripts/modal_retry.sh src/gpu/modal_export.py::parity --model khm-latn 120
10+
set -u
11+
target="$1"; shift
12+
sleep_secs="${!#}" # last arg if numeric
13+
if [[ "$sleep_secs" =~ ^[0-9]+$ ]]; then set -- "${@:1:$#-1}"; else sleep_secs=60; fi
14+
until modal run --detach "$target" "$@"; do
15+
echo "[modal_retry] $target failed — retrying in ${sleep_secs}s ($(date))"
16+
sleep "$sleep_secs"
17+
done
18+
echo "[modal_retry] $target completed"

scripts/watch_marker.sh

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#!/bin/bash
2+
# Watch a Modal volume marker with debounce, then act (or just report).
3+
# The pattern behind every poller this campaign needed.
4+
#
5+
# scripts/watch_marker.sh <volume> <remote_path> [check_interval_secs] [cmd...]
6+
#
7+
# Prints WAITING lines to stderr, exits 0 when the marker appears, then
8+
# runs cmd if given. No debounce needed for a marker that only appears.
9+
set -u
10+
volume="$1"; marker="$2"; interval="${3:-600}"; shift 3 2>/dev/null || shift $#
11+
while true; do
12+
rm -f /tmp/.watch_marker_$$
13+
if modal volume get "$volume" "$marker" "/tmp/.watch_marker_$$" >/dev/null 2>&1; then
14+
[ -f /tmp/.watch_marker_$$ ] && break
15+
fi
16+
sleep "$interval"
17+
done
18+
echo "[watch_marker] $volume:$marker present ($(date))"
19+
if [ "$#" -gt 0 ]; then exec "$@"; fi

src/gpu/modal_distill.py

Lines changed: 61 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,37 @@
5757
CHECKPOINTS = modal.Volume.from_name("rababa-checkpoints")
5858

5959

60+
61+
VOLUME_MOUNTS = {
62+
"rababa": "/checkpoints",
63+
"secryst": "/secryst-checkpoints",
64+
"persian": "/persian-checkpoints",
65+
}
66+
DATA_MOUNTS = {
67+
"secryst": "/secryst-datasets",
68+
"persian": "/persian-datasets",
69+
}
70+
71+
72+
def resolve_spec(spec: dict) -> dict:
73+
"""Volume-relative paths for a spec — the single owner of "which
74+
volume does this teacher/student/dataset live on" (previously four
75+
pasted vol_map blocks)."""
76+
teacher_vol = spec.get("teacher_volume", "rababa")
77+
data_root = spec.get("data_volume", DATA_MOUNTS.get(teacher_vol, "/datasets"))
78+
teacher = (
79+
spec["teacher"] if spec.get("teacher_is_hub")
80+
else str(Path(VOLUME_MOUNTS[teacher_vol]) / spec["teacher"])
81+
)
82+
out_root = str(Path(VOLUME_MOUNTS[spec.get("out_volume", teacher_vol)]) / spec["out"])
83+
return {
84+
"teacher_vol": teacher_vol,
85+
"data_root": data_root,
86+
"teacher": teacher,
87+
"out_root": out_root,
88+
"best": str(Path(out_root) / "best"),
89+
}
90+
6091
def _ensure_src_path() -> None:
6192
# Modal copies the entry file to /root/<name>.py while the repo image
6293
# sits at /root/interscript-ml — cover both layouts before importing
@@ -398,19 +429,10 @@ def evaluate_per(spec_id: str, limit: int = 0) -> dict:
398429
from transformers import AutoModelForSeq2SeqLM, AutoTokenizer
399430

400431
spec = SPECS[spec_id]
401-
teacher_vol = spec.get("teacher_volume", "rababa")
402-
vol_map = {
403-
"rababa": "/checkpoints",
404-
"secryst": "/secryst-checkpoints",
405-
"persian": "/persian-checkpoints",
406-
}
407-
data_vol = {"secryst": "/secryst-datasets",
408-
"persian": "/persian-datasets"}.get(teacher_vol, "/datasets")
409-
data_vol = spec.get("data_volume", data_vol)
410-
teacher_path = (spec["teacher"] if spec.get("teacher_is_hub")
411-
else str(Path(vol_map[teacher_vol]) / spec["teacher"]))
412-
student_vol = vol_map[spec.get("out_volume", teacher_vol)]
413-
student_path = Path(student_vol) / spec["out"] / "best"
432+
paths = resolve_spec(spec)
433+
data_vol = paths["data_root"]
434+
teacher_path = paths["teacher"]
435+
student_path = Path(paths["best"])
414436
test_rel = spec.get("eval_test") or spec.get("test")
415437
if not test_rel:
416438
raise RuntimeError(f"{spec_id}: no test path")
@@ -510,20 +532,9 @@ def distill_sequence(spec_id: str, epochs: int = 3) -> dict:
510532
spec = SPECS[spec_id]
511533
teacher_vol = spec.get("teacher_volume", "rababa")
512534

513-
vol_map = {
514-
"rababa": "/checkpoints",
515-
"secryst": "/secryst-checkpoints",
516-
"persian": "/persian-checkpoints",
517-
}
518-
teacher_root = vol_map[teacher_vol]
519-
out_root_vol = vol_map[spec.get("out_volume", teacher_vol)]
520-
teacher_path = (spec["teacher"] if spec.get("teacher_is_hub")
521-
else str(Path(teacher_root) / spec["teacher"]))
522-
523-
data_vol = {"secryst": "/secryst-datasets",
524-
"persian": "/persian-datasets"}.get(teacher_vol, "/datasets")
525-
data_vol = spec.get("data_volume", data_vol)
526-
train_path = Path(data_vol) / spec["train"]
535+
paths = resolve_spec(spec)
536+
teacher_path = paths["teacher"]
537+
train_path = Path(paths["data_root"]) / spec["train"]
527538

528539
# Teacher: use its OWN tokenizer (sentencepiece for umt5)
529540
teacher_tok = AutoTokenizer.from_pretrained(str(teacher_path))
@@ -644,15 +655,15 @@ def collate(batch):
644655
train_files = [(train_path, unit_limits[0] if unit_limits else 0)]
645656
for i, p in enumerate(spec.get("train_extra", [])):
646657
lim = unit_limits[i + 1] if i + 1 < len(unit_limits) else 0
647-
train_files.append((Path(data_vol) / p, lim))
658+
train_files.append((Path(paths["data_root"]) / p, lim))
648659
train_ds = Pairs(train_files)
649660
print(f"[{spec_id}] train pairs: {len(train_ds)} from {len(train_files)} files", flush=True)
650661
label_beams = int(spec.get("label_beams", 4))
651662

652663
# Step 1: teacher generates labels (beam-4) for the full corpus.
653664
# Resumable: evictions mid-labeling are routine on long jobs —
654665
# already-labeled srcs are skipped, the rest are appended.
655-
out_root = Path(out_root_vol) / spec["out"]
666+
out_root = Path(paths["out_root"])
656667
out_root.mkdir(parents=True, exist_ok=True)
657668
labels_file = spec.get("labels_file", "teacher_labels.jsonl")
658669
if labels_file.endswith(".b64"):
@@ -999,17 +1010,9 @@ def evaluate_der(spec_id: str, window: int = 1400, limit: int = 0) -> dict:
9991010
)
10001011

10011012
spec = SPECS[spec_id]
1002-
vol_map = {
1003-
"rababa": "/checkpoints",
1004-
"secryst": "/secryst-checkpoints",
1005-
"persian": "/persian-checkpoints",
1006-
}
1007-
teacher_path = (spec["teacher"] if spec.get("teacher_is_hub")
1008-
else str(Path(vol_map[spec.get("teacher_volume", "rababa")]) / spec["teacher"]))
1009-
student_path = (
1010-
Path(vol_map[spec.get("out_volume", spec.get("teacher_volume", "rababa"))])
1011-
/ spec["out"] / "best"
1012-
)
1013+
paths = resolve_spec(spec)
1014+
teacher_path = paths["teacher"]
1015+
student_path = Path(paths["best"])
10131016

10141017
tok = AutoTokenizer.from_pretrained("google/byt5-small")
10151018
teacher = AutoModelForSeq2SeqLM.from_pretrained(teacher_path).to("cuda").eval()
@@ -1051,9 +1054,7 @@ def der_ce(model) -> dict:
10511054
# what r7-style _init_choice probes read)
10521055
import json
10531056

1054-
out_root = Path(
1055-
vol_map[spec.get("out_volume", spec.get("teacher_volume", "rababa"))]
1056-
) / spec["out"]
1057+
out_root = Path(paths["out_root"])
10571058
out_root.mkdir(parents=True, exist_ok=True)
10581059
(out_root / "final_eval.json").write_text(
10591060
json.dumps(result, indent=2), encoding="utf-8"
@@ -1103,20 +1104,10 @@ def distill_microkimi(spec_id: str, epochs: int = 3, calib_batches: int = 64,
11031104
)
11041105

11051106
spec = SPECS[spec_id]
1106-
teacher_vol = spec.get("teacher_volume", "rababa")
1107-
vol_map = {
1108-
"rababa": "/checkpoints",
1109-
"secryst": "/secryst-checkpoints",
1110-
"persian": "/persian-checkpoints",
1111-
}
1112-
data_vol = {"secryst": "/secryst-datasets",
1113-
"persian": "/persian-datasets"}.get(teacher_vol, "/datasets")
1114-
data_vol = spec.get("data_volume", data_vol)
1115-
out_root_vol = vol_map[spec.get("out_volume", teacher_vol)]
1116-
teacher_path = (spec["teacher"] if spec.get("teacher_is_hub")
1117-
else str(Path(vol_map[teacher_vol]) / spec["teacher"]))
1118-
out_root = Path(out_root_vol) / spec["out"]
1107+
paths = resolve_spec(spec)
1108+
out_root = Path(paths["out_root"])
11191109
out_root.mkdir(parents=True, exist_ok=True)
1110+
teacher_path = paths["teacher"]
11201111

11211112
student_tok = AutoTokenizer.from_pretrained("google/byt5-small")
11221113
teacher = AutoModelForSeq2SeqLM.from_pretrained(teacher_path).to("cuda").eval()
@@ -1404,7 +1395,6 @@ def qwen_next_chain() -> dict:
14041395
14051396
modal run --detach src/gpu/modal_distill.py::qwen_chain
14061397
"""
1407-
import json
14081398
import time
14091399
from pathlib import Path
14101400

@@ -1414,38 +1404,29 @@ def qwen_next_chain() -> dict:
14141404
("ara-diac-small-muon", "rababa_arabic_distill_small/run-005-muon"),
14151405
("ara-diac-small-2", "rababa_arabic_distill_small/run-006-r7-muon"),
14161406
]
1417-
ROOT = Path("/checkpoints")
1418-
1419-
def log(run: str, event: str) -> None:
1420-
# mkdir: a fresh arm's run dir does not exist until its training
1421-
# creates it — the first watch line must not crash on that
1422-
run_dir = ROOT / run
1423-
run_dir.mkdir(parents=True, exist_ok=True)
1424-
with (run_dir / "chain_log.jsonl").open("a", encoding="utf-8") as fh:
1425-
fh.write(json.dumps({"t": round(time.time()), "event": event}) + "\n")
1426-
CHECKPOINTS.commit()
14271407

1428-
def latest_step(run: str) -> int:
1429-
steps = [int(p.name.split("-")[1]) for p in (ROOT / run).glob("step-*")]
1430-
return max(steps) if steps else -1
1408+
_ensure_src_path()
1409+
from gpu.runstate import RunState
14311410

14321411
status = {}
14331412
for spec_id, run in ARMS:
1434-
while not (ROOT / run / "best" / "config.json").exists():
1413+
state = RunState(Path("/checkpoints") / run)
1414+
while not state.training_done():
14351415
CHECKPOINTS.reload()
1436-
before = latest_step(run)
1437-
log(run, f"watch step={before}")
1416+
before = state.latest_step()
1417+
state.log(f"watch step={before}", commit=CHECKPOINTS.commit)
14381418
time.sleep(1200)
14391419
CHECKPOINTS.reload()
1440-
after = latest_step(run)
1441-
if after == before and not (ROOT / run / "best" / "config.json").exists():
1442-
log(run, f"stalled at step={after}; respawning {spec_id}")
1420+
after = state.latest_step()
1421+
if after == before and not state.training_done():
1422+
state.log(f"stalled at step={after}; respawning {spec_id}",
1423+
commit=CHECKPOINTS.commit)
14431424
distill_sequence.spawn(spec_id, epochs=3)
1444-
log(run, "training complete (best present)")
1445-
if not (ROOT / run / "final_eval.json").exists():
1446-
log(run, "evaluating")
1425+
state.log("training complete (best present)", commit=CHECKPOINTS.commit)
1426+
if not state.eval_done():
1427+
state.log("evaluating", commit=CHECKPOINTS.commit)
14471428
evaluate_der.remote(spec_id=spec_id)
1448-
log(run, "eval done")
1429+
state.log("eval done", commit=CHECKPOINTS.commit)
14491430
status[run] = "complete"
14501431
return status
14511432

src/gpu/runstate.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
"""Run-state markers — the durable protocol every arm, orchestrator, and
2+
supervisor agrees on.
3+
4+
The contract (learned from incidents, now an interface):
5+
- training is done when ``best/config.json`` exists (save_pretrained)
6+
- evaluation is done when ``final_eval.json`` exists (evaluate_der)
7+
- progress is observable as ``step-N`` checkpoint dirs; a stall is no
8+
new step for STALL_SECS while training is incomplete
9+
- ``chain_log.jsonl`` is the append-only audit trail
10+
11+
The mkdir bug that killed the orchestrator's arm 4 (log() opened a file
12+
inside a run dir only training creates) is unrepresentable through this
13+
interface: every writer mkdirs.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
from pathlib import Path
19+
20+
TRAINING_DONE = "best/config.json"
21+
EVAL_DONE = "final_eval.json"
22+
CHAIN_LOG = "chain_log.jsonl"
23+
24+
25+
class RunState:
26+
def __init__(self, root: Path) -> None:
27+
self.root = Path(root)
28+
29+
def training_done(self) -> bool:
30+
return (self.root / TRAINING_DONE).exists()
31+
32+
def eval_done(self) -> bool:
33+
return (self.root / EVAL_DONE).exists()
34+
35+
def latest_step(self) -> int:
36+
steps = [int(p.name.split("-")[1]) for p in self.root.glob("step-*")]
37+
return max(steps) if steps else -1
38+
39+
def log(self, event: str, commit=None) -> None:
40+
import json
41+
import time
42+
43+
self.root.mkdir(parents=True, exist_ok=True)
44+
with (self.root / CHAIN_LOG).open("a", encoding="utf-8") as fh:
45+
fh.write(json.dumps({"t": round(time.time()), "event": event}) + "\n")
46+
if commit is not None:
47+
commit()
48+
49+
def read_eval(self) -> dict | None:
50+
import json
51+
52+
path = self.root / EVAL_DONE
53+
if not path.exists():
54+
return None
55+
return json.loads(path.read_text(encoding="utf-8"))

tests/test_gpu_runstate.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
"""RunState marker protocol tests — pure CPU."""
2+
3+
from __future__ import annotations
4+
5+
import sys
6+
from pathlib import Path
7+
8+
sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src"))
9+
10+
from gpu.runstate import RunState # noqa: E402
11+
12+
13+
def test_fresh_run_state_reports_nothing_done(tmp_path: Path) -> None:
14+
state = RunState(tmp_path / "run-x")
15+
assert not state.training_done()
16+
assert not state.eval_done()
17+
assert state.latest_step() == -1
18+
assert state.read_eval() is None
19+
20+
21+
def test_log_writes_into_missing_run_dir(tmp_path: Path) -> None:
22+
# the arm-4 bug: first log must create the run dir, not crash
23+
state = RunState(tmp_path / "brand-new-arm")
24+
state.log("watch step=-1")
25+
lines = (tmp_path / "brand-new-arm" / "chain_log.jsonl").read_text().splitlines()
26+
assert len(lines) == 1
27+
assert "watch step=-1" in lines[0]
28+
29+
30+
def test_step_and_marker_semantics(tmp_path: Path) -> None:
31+
state = RunState(tmp_path / "run-y")
32+
for n in (500, 2000, 1000):
33+
(tmp_path / "run-y" / f"step-{n}").mkdir(parents=True)
34+
assert state.latest_step() == 2000
35+
(tmp_path / "run-y" / "best").mkdir()
36+
(tmp_path / "run-y" / "best" / "config.json").write_text("{}")
37+
assert state.training_done()
38+
assert not state.eval_done()
39+
40+
41+
def test_read_eval_roundtrip(tmp_path: Path) -> None:
42+
state = RunState(tmp_path / "run-z")
43+
(tmp_path / "run-z").mkdir()
44+
(tmp_path / "run-z" / "final_eval.json").write_text('{"gate_pass": true}')
45+
assert state.eval_done()
46+
assert state.read_eval()["gate_pass"] is True

0 commit comments

Comments
 (0)