Skip to content

Add SLURM-aware exp manager run timer - #16214

Open
pzelasko wants to merge 1 commit into
mainfrom
codex/exp-manager-slurm-max-time
Open

Add SLURM-aware exp manager run timer#16214
pzelasko wants to merge 1 commit into
mainfrom
codex/exp-manager-slurm-max-time

Conversation

@pzelasko

@pzelasko pzelasko commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do ?

Allow exp_manager.max_time_per_run to optionally count from the start of the current SLURM allocation so preprocessing and setup time cannot consume the checkpoint safety margin unnoticed.

Collection: Core / experiment manager

Changelog

  • Add the opt-in max_time_per_run_from_slurm configuration field.
  • Read and validate the SLURM-provided SLURM_JOB_START_TIME UNIX timestamp.
  • Check elapsed allocation time before training and refresh the offset when the monotonic training clock starts.
  • Preserve the existing training-loop-based behavior by default.
  • Add unit coverage for elapsed-time accounting, pre-fit expiry and checkpoint saving, invalid timestamps, opt-in compatibility, and public config wiring.
  • Document configuration and checkpoint-buffer guidance.

Usage

exp_manager:
  max_time_per_run: 00:03:45:00
  max_time_per_run_from_slurm: true

GitHub Actions CI

The changed core files and documentation should select the relevant GitHub Actions checks.

Before your PR is "Ready for review"

Pre checks:

  • Read and followed the contributor guidelines.
  • Added focused regression and edge-case tests.
  • Added user-facing configuration documentation.
  • Optional-install components and import guards are not affected.

PR Type:

  • New Feature
  • Bugfix
  • Documentation

Validation

  • pre-commit run
  • pre-commit run --from-ref origin/main --to-ref HEAD
  • pytest -q --cpu -m unit tests/core_ptl/test_ptl_stateless_timer.py — 13 passed, 1 GPU-only test skipped
  • uv sync --locked --group docs
  • uv run make -C docs html — succeeded; 190 pre-existing optional-import and duplicate-toctree warnings
  • git diff --check origin/main...HEAD

Not run: the full repository and GPU test suites, because the change is isolated to timer bookkeeping and is covered by focused CPU unit tests.

Additional Information

No linked issue.

Signed-off-by: Piotr Żelasko <pzelasko@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@pzelasko

pzelasko commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test 860b0f7

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