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 weights_only for load #5

Merged
merged 4 commits into from
Aug 25, 2024
Merged

Conversation

kit1980
Copy link
Contributor

@kit1980 kit1980 commented Aug 24, 2024

torch.load without weights_only parameter is unsafe. Explicitly set weights_only to False only if you trust the data you load and full pickle functionality is needed, otherwise set weights_only=True.

If weights_only=True doesn't work for some cases, then explicit weights_only=False should be used.

Found with https://github.com/pytorch-labs/torchfix/

Copy link
Member

@francois-rozet francois-rozet left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! This is the very first PR of Azula 🥳

I agree that weights_only=True should be used as much as possible. However, we already set it to true by default, and I am not sure it necessary to enforce it further. It also seems to not be supported by older version of PyTorch (1.12).

Do you know the version it was introduced in?

azula/plugins/adm/__init__.py Outdated Show resolved Hide resolved
@kit1980
Copy link
Contributor Author

kit1980 commented Aug 25, 2024

Do you know the version it was introduced in?

The PR that added the option is pytorch/pytorch#86812, first release with it is PyTorch 1.13.0

@francois-rozet francois-rozet merged commit 39d53fb into probabilists:master Aug 25, 2024
5 checks passed
@francois-rozet
Copy link
Member

francois-rozet commented Aug 25, 2024

Thank you! I have dropped the support for torch<=1.12 to be able to use weights_only everywhere. Merged 👍

I also plan to add a hash-check protocol for our plugins to ensure that downloaded files are safe to read. I have already implemented part of that protocol in #6.

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.

2 participants