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 bnb lora layers not setting active adapter #1294

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

tdrussell
Copy link
Contributor

This appears to be a small oversight introduced with #1106. I was trying to load some loras onto LLMs using text-generation-webui, and noticed that the lora had no effect if the model was loaded with load_in_4bit. But non-quantized models still worked with the lora.

By adding this single line, it matches what is in the standard Linear lora layer. Now the quantized linear lora layers have the lora active by default.

This now matches the behavior of the standard Linear lora layer.
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.

Makes sense to me!
Can you elaborate more on:

the lora layers had no effect when using load_in_4bit

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@BenjaminBossan BenjaminBossan 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 for this bugfix, LGTM. @younesbelkada I think it means that there was no active adapter when using bnb layers.

@younesbelkada
Copy link
Contributor

Ok perfect, we can probably merge it then

@BenjaminBossan BenjaminBossan merged commit c4cf9e7 into huggingface:main Jan 2, 2024
14 checks passed
@tdrussell tdrussell deleted the fix_bnb_active_adapter branch January 6, 2024 16:39
BenjaminBossan added a commit to BenjaminBossan/peft that referenced this pull request Jan 11, 2024
Resolves huggingface#1345

See also huggingface#1294 for a similar (but incomplete) fix.

This commit fixes the setting of the adapter name on a couple of
quantized layers that was accidentally removed in huggingface#1106. This affects
users who use a non-default adapter name when they want to train these
layers.
BenjaminBossan added a commit that referenced this pull request Jan 12, 2024
Resolves #1345

See also #1294 for a similar (but incomplete) fix.

This commit fixes the setting of the adapter name on a couple of
quantized layers that was accidentally removed in #1106. This affects
users who use a non-default adapter name when they want to train these
layers.

---------

Co-authored-by: Sourab Mangrulkar <13534540+pacman100@users.noreply.github.com>
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.

4 participants