Skip to content
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

Fix PeftModel.disable_adapter #644

Merged
merged 1 commit into from Jun 27, 2023
Merged

Conversation

ain-soph
Copy link
Contributor

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

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

Copy link
Collaborator

@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 🤗

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 27, 2023

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

Copy link
Collaborator

@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
11 checks passed
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.

None yet

4 participants