Skip to content

Conversation

sgugger
Copy link
Collaborator

@sgugger sgugger commented Aug 24, 2023

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.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 24, 2023

The documentation is not available anymore as the PR was closed or merged.

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)
Copy link
Collaborator Author

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,
Copy link
Collaborator Author

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)
Copy link
Collaborator Author

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",
Copy link
Collaborator Author

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.

@sgugger sgugger requested a review from LysandreJik August 24, 2023 09:55
Copy link
Collaborator

@ArthurZucker ArthurZucker left a 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) 😅

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! Thanks for the cleanup!

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@sgugger sgugger merged commit 2febd50 into main Aug 24, 2023
@sgugger sgugger deleted the min_calls branch August 24, 2023 12:56
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
…face#25715)

* Fix number of minimal calls to the Hub with peft integration

* Alternate design

* And this way?

* Revert

* Address comments
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.

5 participants