Skip to content

Fix PeftModel.disable_adapter #644

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.

Sign up for GitHub

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

Jump to bottom
Merged
younesbelkada merged 1 commit into from
Jun 27, 2023
Merged

Fix PeftModel.disable_adapter #644

younesbelkada merged 1 commit into from
Jun 27, 2023

Conversation

@ain-soph
Copy link
Contributor

@ain-soph ain-soph commented Jun 27, 2023

According to the init method, self.peft_config is always a dict. It'll be something like {"default": PeftConfig Instance}.

peft/src/peft/peft_model.py

Lines 97 to 113 in f5352f0

def __init__(self, model, peft_config: PeftConfig, adapter_name="default"):
super().__init__()
self.base_model = model
self.config = self.base_model.config
self.modules_to_save = None
self.peft_config = {}
self.active_adapter = adapter_name
self.peft_type = peft_config.peft_type
self.base_model_torch_dtype = getattr(model, "dtype", None)
if not isinstance(peft_config, PromptLearningConfig):
self.peft_config[adapter_name] = peft_config
self.base_model = PEFT_TYPE_TO_MODEL_MAPPING[peft_config.peft_type](
self.base_model, self.peft_config, adapter_name
)
self.set_additional_trainable_modules(peft_config, adapter_name)
else:
self.add_adapter(adapter_name, peft_config)

Therefore, existing disable_adapter method is checking the wrong item. It should check self.peft_config["default"] instead of self.peft_config

peft/src/peft/peft_model.py

Lines 366 to 381 in f5352f0

def disable_adapter(self):
"""
Disables the adapter module.
"""
try:
if isinstance(self.peft_config, PromptLearningConfig):
old_forward = self.forward
self.forward = self.base_model.forward
else:
self.base_model.disable_adapter_layers()
yield
finally:
if isinstance(self.peft_config, PromptLearningConfig):
self.forward = old_forward
else:
self.base_model.enable_adapter_layers()

pacman100
pacman100 approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@pacman100 pacman100 left a comment

Choose a reason for hiding this comment

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

Thank you @ain-soph for the fixes, LGTM! Nice catch 🤗

@pacman100 pacman100 requested a review from younesbelkada June 27, 2023 17:49
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 27, 2023 •
edited
Loading

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

younesbelkada
younesbelkada approved these changes Jun 27, 2023
View reviewed changes
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.

great catch indeed ! Thanks for fixing @ain-soph !

@younesbelkada younesbelkada merged commit d9b0a11 into huggingface:main Jun 27, 2023
Guy-Bilitski pushed a commit to Guy-Bilitski/peft that referenced this pull request May 13, 2025
@ain-soph
Update peft_model.py (huggingface#644)
743c3fb
cyyever pushed a commit to cyyever/peft that referenced this pull request Sep 4, 2025
@tannonk
Handle potentially long sequences with DataCollatorForCompletionOnlyLM (
029f961 
huggingface#644)

* avoid RuntimeError on long sequences

* add unittests and format

* remove dependency on external repo

* bug fix in DataCollatorForCompletionOnlyLM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@pacman100 pacman100 pacman100 approved these changes

@younesbelkada younesbelkada younesbelkada approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ain-soph @HuggingFaceDocBuilderDev @pacman100 @younesbelkada