Skip to content

fix(train): save checkpoint when Slurm kills the job, resume on restart - #621

Closed
RyanPCo wants to merge 1 commit into
ryanco/delete-actfrom
ryanco/phase1-code-fixes
Closed

RyanPCo wants to merge 1 commit into
ryanco/delete-actfrom
ryanco/phase1-code-fixes

Conversation

@RyanPCo

@RyanPCo RyanPCo commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor
  • pl_utils/preemption.py: PreemptionCheckpoint callback. On the signal submitit
    sends (USR2 / $SUBMITIT_PREEMPT_SIGNAL) it saves /checkpoints/last.ckpt
    (what trainHydra resumes from on SLURM_RESTART_COUNT) and then chains to the
    previously installed handler (submitit's requeue). Restores the old handler
    on teardown. Details that matter:
    • before saving it points every ModelCheckpoint(save_last=True) in the same
      directory (compared via realpath; Lightning realpaths dirpath) at
      last.ckpt, otherwise their version counter writes every later periodic
      save to last-v1.ckpt, ... (also after resume, since the path is part of
      ModelCheckpoint's saved state) while trainHydra resumes the stale file;
    • writes last.ckpt.preempt and renames, so a GraceTime SIGKILL mid-write
      cannot leave a truncated last.ckpt;
    • a failed save is logged and the requeue still runs;
    • forked DataLoader workers inherit the handler (Slurm signals every
      process in the step); they return immediately instead of saving.
      There is deliberately no self-requeue for non-submitit launches: nothing in
      the repo launches training that way, and a raw sbatch requeue would
      recompose hydra.run.dir with a new timestamp and never find last.ckpt.
  • trainHydra: SLURMEnvironment(auto_requeue=False) + PreemptionCheckpoint.
    Lightning's own handler listened on SIGUSR1, which nothing ever sent, and
    Lightning skips registering on a signal submitit already owns. On requeue,
    resume only if last.ckpt exists (preemption during dataset setup happens
    before the callback is installed); warn and start fresh otherwise.
  • hydra/launcher/submitit*.yaml: signal_delay_s: 600 (launcher default 120 s is
    too short to write a multi-GB pi0.5 checkpoint to NFS before the time-limit
    SIGKILL). Preemption itself is bounded by the partition GraceTime (300 s on
    overcap), which no launcher setting changes.
  • pl_model: torch.distributed.barrier() in on_fit_start / on_validation_end is
    now a no-op without a process group (CPU smoke test, devices=1 non-DDP).
  • tests/unit: test_preemption (8: chain, default path, env signal, save
    failure, no temp file, ModelCheckpoint coexistence across a resume, the same
    through a symlinked run dir, forked worker does nothing),
    test_pl_model_barriers (2).

Co-Authored-By: Claude Fable 5.1 [email protected]
Claude-Session: https://claude.ai/code/session_01UHDiN8yfERcsiFdz4mXuZi

🤖 Generated with Claude Code

https://claude.ai/code/session_01UHDiN8yfERcsiFdz4mXuZi

@RyanPCo RyanPCo mentioned this pull request Sep 12, 2026

RyanPCo commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor Author

@RyanPCo
RyanPCo marked this pull request as ready for review September 12, 2026 17:30
@RyanPCo
RyanPCo force-pushed the ryanco/phase1-code-fixes branch from 062bd40 to 23b5c36 Compare September 12, 2026 19:39
@RyanPCo
RyanPCo force-pushed the ryanco/tests-dir branch 2 times, most recently from 2728174 to 565f8a1 Compare September 12, 2026 21:28
@RyanPCo
RyanPCo force-pushed the ryanco/phase1-code-fixes branch from 23b5c36 to eae89d6 Compare September 12, 2026 21:28
@RyanPCo
RyanPCo changed the base branch from ryanco/tests-dir to graphite-base/621 September 12, 2026 21:32
@RyanPCo
RyanPCo force-pushed the ryanco/phase1-code-fixes branch from eae89d6 to 3db4d2e Compare September 12, 2026 21:32
@RyanPCo
RyanPCo changed the base branch from graphite-base/621 to ryanco/delete-act September 12, 2026 21:32
@RyanPCo RyanPCo changed the title fix(train): checkpoint on Slurm preemption, guard barriers, delete dead ACT fix(train): checkpoint on Slurm preemption, guard barriers Sep 12, 2026
@RyanPCo RyanPCo changed the title fix(train): checkpoint on Slurm preemption, guard barriers fix(train): save checkpoint when Slurm kills the job, resume on restart Sep 12, 2026
@RyanPCo
RyanPCo marked this pull request as draft September 12, 2026 22:04
- pl_utils/preemption.py: PreemptionCheckpoint callback. On the signal submitit
  sends (USR2 / $SUBMITIT_PREEMPT_SIGNAL) it saves <root>/checkpoints/last.ckpt
  (what trainHydra resumes from on SLURM_RESTART_COUNT) and then chains to the
  previously installed handler (submitit's requeue). Restores the old handler
  on teardown. Details that matter:
  * before saving it points every ModelCheckpoint(save_last=True) in the same
    directory (compared via realpath; Lightning realpaths dirpath) at
    last.ckpt, otherwise their version counter writes every later periodic
    save to last-v1.ckpt, ... (also after resume, since the path is part of
    ModelCheckpoint's saved state) while trainHydra resumes the stale file;
  * writes last.ckpt.preempt and renames, so a GraceTime SIGKILL mid-write
    cannot leave a truncated last.ckpt;
  * a failed save is logged and the requeue still runs;
  * forked DataLoader workers inherit the handler (Slurm signals every
    process in the step); they return immediately instead of saving.
  There is deliberately no self-requeue for non-submitit launches: nothing in
  the repo launches training that way, and a raw sbatch requeue would
  recompose hydra.run.dir with a new timestamp and never find last.ckpt.
- trainHydra: SLURMEnvironment(auto_requeue=False) + PreemptionCheckpoint.
  Lightning's own handler listened on SIGUSR1, which nothing ever sent, and
  Lightning skips registering on a signal submitit already owns. On requeue,
  resume only if last.ckpt exists (preemption during dataset setup happens
  before the callback is installed); warn and start fresh otherwise.
- pl_model: torch.distributed.barrier() in on_fit_start / on_validation_end is
  now a no-op without a process group (CPU smoke test, devices=1 non-DDP).
- tests/unit: test_preemption (8: chain, default path, env signal, save
  failure, no temp file, ModelCheckpoint coexistence across a resume, the same
  through a symlinked run dir, forked worker does nothing),
  test_pl_model_barriers (2).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01UHDiN8yfERcsiFdz4mXuZi
@RyanPCo

RyanPCo commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Not doing preemption checkpointing (decided 2026-09-13); see
docs/critique/00_README.md Phase 1.

@RyanPCo RyanPCo closed this Sep 13, 2026
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.

1 participant