Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change how TorchFT manages user_state_dict #87

Merged
merged 5 commits into from
Jan 30, 2025
Merged

Conversation

fegin
Copy link
Contributor

@fegin fegin commented Jan 29, 2025

This PR closes some state_dict gaps when integrating with TorchTitan:

  1. User state_dict() and load_state_dict() functions can be initialized lazily.
  2. Change weights_only to False for torch.load as we may have to load some non-tensor states.

This PR closes some state_dict gaps when integrating with TorchTitan:
1. User state_dict() and load_state_dict() functions can be initialized lazily.
2. Change weights_only to False for torch.load as we may have to load some non-tensor states.
@fegin fegin requested review from d4l3k and H-Huang January 29, 2025 19:31
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 29, 2025
Copy link
Member

@d4l3k d4l3k left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -172,7 +172,9 @@ def load_from_address(cls, address: str, timeout: timedelta) -> T:
data = f.read()

reader = io.BytesIO(data)
return torch.load(reader, weights_only=True)
# We have to set weights_only to True as there are some non-tensor
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# We have to set weights_only to True as there are some non-tensor
# We have to set weights_only to False as there are some non-tensor

@fegin fegin merged commit 6e4ae38 into main Jan 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants