Skip to content

Fix number of minimal calls to the Hub with peft integration#25715

Merged
sgugger merged 5 commits into
mainfrom
min_calls
Aug 24, 2023
Merged

Fix number of minimal calls to the Hub with peft integration#25715
sgugger merged 5 commits into
mainfrom
min_calls

Conversation

@sgugger

@sgugger sgugger commented Aug 24, 2023

Copy link
Copy Markdown
Collaborator

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

HuggingFaceDocBuilderDev commented Aug 24, 2023

Copy link
Copy Markdown

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

Comment thread 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)

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

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) 😅

Comment thread src/transformers/utils/peft_utils.py
Comment thread src/transformers/modeling_utils.py
Comment thread src/transformers/modeling_utils.py

@LysandreJik LysandreJik left a comment

Copy link
Copy Markdown
Member

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!

@younesbelkada younesbelkada left a comment

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown

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