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

Torch: test on 2.0 and latest versions + explicitly load with weights_only=True #2488

Merged
merged 6 commits into from
Aug 26, 2024

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Aug 26, 2024

To merge before #2440 (needed to test it). Two things in this PR:

  1. we are now running the torch-test CI with torch~=2.0 and torch=="latest". This is to ensure a sense of backward-compatibility and still testing on latest updates. 2.0 was chosen arbitrarily. It has been released ~15 months ago. It doesn't mean huggingface_hub does not support torch 1.0 (we would have received reports^^), just that we are not guaranteeing it.

  2. Pytorch tests are currently failing in CI with this FutureWarning:

FutureWarning: You are using torch.load with weights_only=False (the current default value), which uses the default pickle module implicitly. It is possible to construct malicious pickle data which will execute arbitrary code during unpickling (See /~https://github.com/pytorch/pytorch/blob/main/SECURITY.md#untrusted-models for more details). In a future release, the default value for weights_only will be flipped to True. This limits the functions that could be executed during unpickling. Arbitrary objects will no longer be allowed to be loaded via this mode unless they are explicitly allowlisted by the user via torch.serialization.add_safe_globals. We recommend you start setting weights_only=True for any use case where you don't have full control of the loaded file. Please open an issue on GitHub for any issues related to this experimental feature.

This is actually a good opportunity to add weights_only=True explicitly in the Pytorch hub mixin when loading from an unsafe pickle. Note that the default file type is still .safetensors when saving / loading weights.

Expectations: a green CI and relaxed users!

@Wauplin Wauplin requested a review from LysandreJik August 26, 2024 13:00
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Wauplin Wauplin changed the title [Mixin] Load unsafe pickle with explicit weights_only=True Torch: test on 2.0 and latest versions + explicitly load with weights_only=True Aug 26, 2024
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Ok, LGTM!

FYI in transformers we support torch versions that were released up to 24 months ago, which means support for up to torch 1.11: /~https://github.com/huggingface/transformers/pull/28207/files

It would be good to align but if 2.0 is a hard requirement it's not the end of the world!

@Wauplin
Copy link
Contributor Author

Wauplin commented Aug 26, 2024

It's not a hard requirements on our side no. I just needed to chose one but happy to align with transformers on torch==1.11. I've pushed a3a3b45 in that sense. I'll merge once CI is green.

@Wauplin
Copy link
Contributor Author

Wauplin commented Aug 26, 2024

Tests are passing on torch 1.11 so all good to test this one in the CI!

@Wauplin Wauplin merged commit 19a149e into main Aug 26, 2024
19 checks passed
@Wauplin Wauplin deleted the pytorch-mixin-load-weights-only branch August 26, 2024 15:41
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.

3 participants