-
Notifications
You must be signed in to change notification settings - Fork 990
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
Deal with shared memory scenarios #2136
Conversation
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! I know that in the save_model
in accelerate and in save_pretrained
in transformers, we remove the shared tensors in a different way. I don't know how comparable it is to this method in safetensors but my guess it that it is pretty similar.
Good call, I’ll refactor this into a function so that it only warns, doesn’t actually raise an err :) I think logically it should work the same, bar the bnb explicit part |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for iterating ! LGTM !
What does this PR do?
We need to deal with shared memory scenarios when saving with
safetensors
and a model. Since thesafetensor
version relies on passing the entire model instead of the state dict directly, see here, to deal with this, for now a raw copy/paste of the code is done instead to remove duplicate names.(All
state_dict
in torch areOrderedDict
, hence the check)Fixes # (issue)
Two failures on the transformers CI
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@SunMarc