-
Notifications
You must be signed in to change notification settings - Fork 30.9k
Fix number of minimal calls to the Hub with peft integration #25715
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
Conversation
The documentation is not available anymore as the PR was closed or merged. |
src/transformers/utils/peft_utils.py
Outdated
raise ValueError("PEFT is not installed. Please install it with `pip install peft`") | ||
|
||
is_peft_version_compatible = version.parse(importlib.metadata.version("peft")) <= version.parse(min_version) | ||
is_peft_version_compatible = version.parse(importlib.metadata.version("peft")) > version.parse(min_version) |
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.
This part will be reverted and rebased once #25710 is merged.
subfolder: str = None, | ||
token: Optional[str] = None, | ||
commit_hash: Optional[str] = None, | ||
cache_dir: Optional[Union[str, os.PathLike]] = None, |
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.
Add all standard hub kwargs here.
kwargs["_from_auto"] = True | ||
kwargs["name_or_path"] = pretrained_model_name_or_path | ||
trust_remote_code = kwargs.pop("trust_remote_code", None) | ||
code_revision = kwargs.pop("code_revision", None) |
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.
Not strictly related to the issue at hand, but this cleanup makes sure code_revision
is not recursively passed along.
kwargs["_from_auto"] = True | ||
hub_kwargs_names = [ | ||
"cache_dir", | ||
"code_revision", |
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.
This is not strictly a hub kwarg, it's just for the remote code integration.
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.
I think we can reduce the number of calls once more (might be wrong) 😅
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.
Ok, LGTM! Thanks for the cleanup!
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.
Thanks a lot!
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
…face#25715) * Fix number of minimal calls to the Hub with peft integration * Alternate design * And this way? * Revert * Address comments
What does this PR do?
This PR makes sure we don't add two calls to every model instantiation when PEFT is installed by moving the calls to
find_adapter_config_file
after the config is created so we can use the commit hash.