Skip to content

Use monotonic time for training time limits - #3842

Open
betacatsling wants to merge 1 commit into
pytorch:masterfrom
betacatsling:fix/monotonic-training-limit
Open

betacatsling wants to merge 1 commit into
pytorch:masterfrom
betacatsling:fix/monotonic-training-limit

Conversation

@betacatsling

Copy link
Copy Markdown

Description:

TimeLimit measures elapsed time using the wall clock. A clock synchronization or manual clock change can prematurely stop training or extend a configured training limit.

Use time.monotonic for the start and elapsed measurements. Add engine-level regressions for forward and backward wall-clock changes.

Validation: 2 regression failures before the fix; all 5 tests in tests/ignite/handlers/test_time_limit.py pass on CPU afterward. Ruff lint and format checks pass for changed files.

Check list:

  • Regression tests added.
  • No public API or documentation changes required.

Implementation and validation were performed with AI assistance.

Fixes #3841.

@github-actions github-actions Bot added the module: handlers Core Handlers module label Sep 12, 2026

@siddhant-shahhh siddhant-shahhh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TimeLimit measured elapsed time with time.time(), which moves whenever the system clock does. time.monotonic() is the right call for a duration.

Checked the blast radius first: TimeLimit.start_time isn't referenced anywhere else in ignite/ or the tests, and the class has no state_dict or serialization, so nothing outside reads it as a wall-clock timestamp.

Ran the new test both ways. On this branch the file collects 5 and all pass. Revert time_limit.py to master but keep the test, and both parametrisations fail:

test_time_limit_ignores_wall_clock_adjustments[1000-2-False]   assert True is False
test_time_limit_ignores_wall_clock_adjustments[-1000-11-True]  assert False is True

So it catches both directions: a forward clock jump no longer stops training early, and a backward jump no longer blocks termination. Monkeypatching the module's time with a SimpleNamespace exposing both time and monotonic is a nice way to move the two clocks independently.

ruff check and ruff format are clean on both files.

One out-of-scope note, nothing needed here: ignite/handlers/timing.py and Engine's epoch/run duration accounting still use time.time(), so reported durations have the same clock-jump sensitivity.

LGTM.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: handlers Core Handlers module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use monotonic time for training time limits

2 participants