Skip to content

Commit 17433c7

Browse files
committed
Give CheckpointJob the training state it already owned
_has_training_job_completed and _mark_training_job_completed read nothing but the job, and the summary re-derived the legacy rule from physics_backend rather than asking. They become is_trained and mark_trained, so the completion marker sits next to the only state that explains it and the legacy branch is stated once. is_trained is now consistent between training and reporting: a legacy run predates the marker, so requiring one there would retrain every published legacy checkpoint. Tests pin both halves of that rule. The collect locals follow the declared-checkpoint vocabulary.
1 parent eda9b46 commit 17433c7

2 files changed

Lines changed: 62 additions & 26 deletions

File tree

scripts/tools/test/test_train_and_publish_checkpoints.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,37 @@ def test_legacy_collection_preserves_task_directory(tmp_path: Path) -> None:
120120
assert path == str(tmp_path / "rsl_rl" / "Isaac-Test" / "checkpoint.pt")
121121

122122

123+
def test_core_job_needs_the_completion_marker_to_count_as_trained(
124+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
125+
) -> None:
126+
"""A run killed after writing a checkpoint must be retried, not skipped."""
127+
monkeypatch.chdir(tmp_path)
128+
job = CheckpointJob(
129+
workflow="rsl_rl", task_name="Isaac-Test", physics_backend="newtonmjwarp", render_backend="none"
130+
)
131+
run_dir = Path(job.log_root) / "2026-01-01_00-00-00"
132+
run_dir.mkdir(parents=True)
133+
(run_dir / "model_0.pt").write_bytes(b"")
134+
135+
assert job.has_finished
136+
assert not job.is_trained
137+
138+
job.mark_trained()
139+
140+
assert job.is_trained
141+
142+
143+
def test_legacy_job_counts_as_trained_without_the_marker(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
144+
"""Legacy runs predate the marker, so requiring one would retrain every published checkpoint."""
145+
monkeypatch.chdir(tmp_path)
146+
job = CheckpointJob(workflow="rsl_rl", task_name="Isaac-Test")
147+
run_dir = Path(job.log_root) / "2026-01-01_00-00-00"
148+
run_dir.mkdir(parents=True)
149+
(run_dir / "model_0.pt").write_bytes(b"")
150+
151+
assert job.is_trained
152+
153+
123154
def test_publish_uses_collected_checkpoint_without_training_logs(
124155
tmp_path: Path,
125156
capsys: pytest.CaptureFixture[str],

scripts/tools/train_and_publish_checkpoints.py

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,29 @@ def job_id(self) -> str:
111111
return f"{self.workflow}:{self.task_name}"
112112
return f"{self.workflow}:{self.task_name}:{self.physics_backend}:{self.render_backend}"
113113

114+
@property
115+
def is_trained(self) -> bool:
116+
"""Whether this job's training finished and left a checkpoint.
117+
118+
A core job must also carry the marker its training subprocess writes, so a run killed
119+
after writing a checkpoint is retried. Legacy jobs predate the marker.
120+
"""
121+
if self.is_legacy:
122+
return self.has_finished
123+
run_path = self.latest_run
124+
if run_path is None or not os.path.isfile(os.path.join(run_path, _TRAINING_COMPLETE_FILENAME)):
125+
return False
126+
return self.has_finished
127+
128+
def mark_trained(self) -> None:
129+
"""Record that the latest training subprocess exited successfully."""
130+
run_path = self.latest_run
131+
if run_path is None:
132+
raise RuntimeError(f"Unable to determine the latest run for {self.job_id}")
133+
marker_path = os.path.join(run_path, _TRAINING_COMPLETE_FILENAME)
134+
with open(marker_path, "w", encoding="utf-8") as marker_file:
135+
marker_file.write(f"{self.job_id}\n")
136+
114137
@property
115138
def preset_args(self) -> list[str]:
116139
"""Return typed preset selectors for this job."""
@@ -437,27 +460,9 @@ def _run_command(command: list[str], dry_run: bool) -> int:
437460
return subprocess.run(command, check=False, cwd=_REPO_ROOT, env=env).returncode
438461

439462

440-
def _has_training_job_completed(job: CheckpointJob) -> bool:
441-
"""Return whether the latest run exited successfully with a checkpoint."""
442-
run_path = job.latest_run
443-
if run_path is None or not os.path.isfile(os.path.join(run_path, _TRAINING_COMPLETE_FILENAME)):
444-
return False
445-
return job.has_finished
446-
447-
448-
def _mark_training_job_completed(job: CheckpointJob) -> None:
449-
"""Record that the latest training subprocess exited successfully."""
450-
run_path = job.latest_run
451-
if run_path is None:
452-
raise RuntimeError(f"Unable to determine the latest run for {job.job_id}")
453-
marker_path = os.path.join(run_path, _TRAINING_COMPLETE_FILENAME)
454-
with open(marker_path, "w", encoding="utf-8") as marker_file:
455-
marker_file.write(f"{job.job_id}\n")
456-
457-
458463
def train_job(job: CheckpointJob, args: argparse.Namespace, smoke: bool = False) -> bool:
459464
"""Train or smoke-test one checkpoint job."""
460-
if not smoke and not args.force and _has_training_job_completed(job):
465+
if not smoke and not args.force and job.is_trained:
461466
print(f"Skipping completed training job {job.job_id}")
462467
return True
463468

@@ -470,7 +475,7 @@ def train_job(job: CheckpointJob, args: argparse.Namespace, smoke: bool = False)
470475
if not job.has_finished:
471476
print(f"Training did not produce a checkpoint for {job.job_id}", file=sys.stderr)
472477
return False
473-
_mark_training_job_completed(job)
478+
job.mark_trained()
474479
return True
475480

476481

@@ -490,13 +495,13 @@ def collect_pretrained_checkpoint(job: CheckpointJob, output_dir: str, dry_run:
490495
os.makedirs(os.path.dirname(destination), exist_ok=True)
491496
shutil.copy2(source_path, destination)
492497
for checkpoint in job.checkpoints:
493-
aux_source = job.trained_path(checkpoint)
494-
if aux_source is None:
498+
declared_source = job.trained_path(checkpoint)
499+
if declared_source is None:
495500
print(f"No {checkpoint.name} checkpoint matched {checkpoint.run_glob!r} for {job.job_id}")
496501
continue
497-
aux_destination = job.collected_path(output_dir, checkpoint)
498-
print(f"Collecting {aux_source} -> {aux_destination}")
499-
shutil.copy2(aux_source, aux_destination)
502+
declared_destination = job.collected_path(output_dir, checkpoint)
503+
print(f"Collecting {declared_source} -> {declared_destination}")
504+
shutil.copy2(declared_source, declared_destination)
500505
return destination
501506

502507

@@ -582,7 +587,7 @@ def publish_pretrained_checkpoint(job: CheckpointJob, args: argparse.Namespace)
582587
def _summary_row(job: CheckpointJob, output_dir: str) -> list[str | bool]:
583588
"""Return one CSV summary row."""
584589
has_run = job.has_run
585-
has_finished = _has_training_job_completed(job) if job.physics_backend is not None else job.has_finished
590+
has_finished = job.is_trained
586591
collected_path = job.collected_path(output_dir)
587592
review = job.review
588593
return [

0 commit comments

Comments
 (0)