[checkpoint] Log when initial load is skipped on resume - #4635
Merged
fegin merged 5 commits intoSep 16, 2026
Merged
Conversation
YeonwooSung
requested review from
fegin,
tianyu-l,
wconstab and
wwwjn
as code owners
September 12, 2026 08:06
fegin
requested changes
Sep 14, 2026
fegin
reviewed
Sep 15, 2026
fegin
left a comment
Contributor
There was a problem hiding this comment.
LGTM, but tests/unit_tests/cpu/test_checkpoint.py seems to be broken by this change. Please check.
Resuming from checkpoint.folder is the fault-tolerance path and still wins over initial_load_in_hf / initial_load_path. Tell the user those options were ignored. Addresses pytorch#1900.
Empty-folder loads must not warn. Resume warnings must render the actual step, not the sentinel -1.
Resuming from checkpoint.folder is the fault-tolerance restart path, and it runs on every automatic restart of a long job. That is expected behavior, not a misconfiguration, so a warning on each restart is noise. The existing warnings in this file cover settings that can never take effect (load_only with enable_first_step_checkpoint, initial_load_model_only without initial_load_path); this one is different in kind. The message itself is unchanged, so the skip is still visible in the log, which is what pytorch#1900 needed. Tests now assert the skip message is emitted exactly once at info level and never as a warning. They match on the rendered message rather than on the last call, because the load path emits other info lines.
The tests name the checkpoint folder after the running test, so the folder path contains "initial_load" and the substring filter also matched "Loading the checkpoint from <path>." That made the positive tests see two skip messages instead of one. Filter on "ignoring initial_load_path" instead. The phrase has spaces, so a path cannot match it. Moving the filter into the helper also drops the duplicated comprehensions at the call sites.
YeonwooSung
force-pushed
the
pr/warn-initial-load-skipped-on-resume
branch
from
September 16, 2026 02:33
6afaf98 to
92adda7
Compare
|
|
TestBaseCheckpointManagerTracing builds the manager as Mock(spec=BaseCheckpointManager). The spec only covers class attributes, and initial_load_path / initial_load_in_hf / initial_load_in_hf_quantized are set in __init__, so reading them from load() raised AttributeError. Set all three on the fixture. The values keep the new branch untaken, so the trace-span ordering and no_grad assertions are unaffected.
fegin
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resuming from
checkpoint.folderis the fault-tolerance path and still wins overinitial_load_in_hf/initial_load_path. Users who set those options got no signal at all.Log the skip at info level when a folder checkpoint is loaded and any
initial_*option is set. Precedence is unchanged.This is the normal restart path rather than a misconfiguration, so it is logged, not warned about (per review). The existing warnings in this file cover settings that can never take effect:
load_onlywithenable_first_step_checkpoint, andinitial_load_model_onlywithoutinitial_load_path.Addresses #1900.
Test plan
initial_load_pathinitial_load_pathand forinitial_load_in_hftests/unit_tests/cpu/test_checkpoint.py