Skip to content

Enable load_weights_only in checkpointing - #1996

Open
j316chuck wants to merge 5 commits into
NovaSky-AI:mainfrom
j316chuck:chuck-codex/optimizer-load-state
Open

Enable load_weights_only in checkpointing#1996
j316chuck wants to merge 5 commits into
NovaSky-AI:mainfrom
j316chuck:chuck-codex/optimizer-load-state

Conversation

@j316chuck

@j316chuck j316chuck commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve Tinker's optimizer flag through the load-weights API, engine, and JAX/SkyRL-Train backends.
  • Restore optimizer and scheduler state only for load_state_with_optimizer(); weights-only loads leave the live optimizer untouched.

Fixes #1993.

Testing

uv run --extra tinker --extra dev pytest tests/tinker/test_api_validation.py::test_load_weights_request_preserves_optimizer_choice tests/tinker/test_engine.py::test_process_load_weights_forwards_optimizer_choice
uv run --extra tinker --extra skyrl-train --extra ray --extra dev pytest tests/backends/skyrl_train/test_checkpoint_loading.py

All seven focused API, engine, and checkpoint-loading cases pass.

@j316chuck
j316chuck marked this pull request as ready for review August 6, 2026 04:57
@j316chuck j316chuck changed the title Honor optimizer choice when loading Tinker state Enable load_weights_only in checkpointing Aug 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an "optimizer" boolean flag to the checkpoint loading process across multiple backends (JAX, Ray JAX, and SkyRL Train) and exposes it via the Tinker API, enabling optional restoration of optimizer and scheduler states. The feedback highlights two main areas of improvement: first, the "optimizer" field in "LoadWeightsRequest" and "LoadWeightsInput" should default to "False" to preserve backward compatibility with older clients; second, a safety check should be added in the JAX backend to prevent a potential "KeyError" if "optimizer_state" is missing from the checkpoint data.

Comment thread skyrl/tinker/api.py Outdated
class LoadWeightsRequest(BaseModel):
model_id: str
path: str
optimizer: bool

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Adding optimizer as a required boolean field in LoadWeightsRequest without a default value breaks backward compatibility with older clients or SDKs that do not send this field in their payload. Providing a default value (e.g., False) ensures that the API remains backward-compatible and robust against missing fields.

Suggested change
optimizer: bool
optimizer: bool = False

Comment thread skyrl/tinker/types.py Outdated
Comment thread skyrl/backends/jax.py Outdated
Comment thread skyrl/tinker/api.py Outdated
Comment thread skyrl/tinker/types.py
Signed-off-by: Charles Tang <j316chuck@users.noreply.github.com>
Signed-off-by: Charles Tang <j316chuck@users.noreply.github.com>
@j316chuck
j316chuck force-pushed the chuck-codex/optimizer-load-state branch 2 times, most recently from f618708 to 99ca9e0 Compare August 10, 2026 19:35
@j316chuck
j316chuck force-pushed the chuck-codex/optimizer-load-state branch from 6355b87 to d7240b6 Compare August 10, 2026 19:46
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.

Support weights-only checkpoint loading

2 participants