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

Use safetensors by default for PyTorchModelHubMixin class #1989

Closed
NielsRogge opened this issue Jan 18, 2024 · 3 comments · Fixed by #2033
Closed

Use safetensors by default for PyTorchModelHubMixin class #1989

NielsRogge opened this issue Jan 18, 2024 · 3 comments · Fixed by #2033

Comments

@NielsRogge
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The PyTorchModelHubMixin currently saves weights as a pytorch_model.bin file, which is considered unsafe.

Describe the solution you'd like

Would be great to update to leverage the safetensors format by default.

Describe alternatives you've considered

/

Additional context

/

@bmuskalla
Copy link
Contributor

Pushed a potential fix as #2033 - the big open question is whether to keep this backwards-compatible and/or issue a warning if the mixin is still loading it from a pickle file

@Wauplin
Copy link
Contributor

Wauplin commented Feb 20, 2024

Thanks @bmuskalla! I continued the discussion on the PR directly. General guidelines is to prevent unannounced breaking changes as much as possible. So what we should do is to support safetensors right now and after a few releases, make it the default + slowly remove support for unsafe loading.

@Wauplin
Copy link
Contributor

Wauplin commented Feb 26, 2024

@NielsRogge this issue has been completed by @bmuskalla in #2033 and will be available in the soon-to-be-published release! Together with #2001 it fixes the biggest problems you've mentioned about the mixin right? (except handling sharding that should be done soon now that #1938 have been merged)

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 a pull request may close this issue.

3 participants