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.

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

PEFT Integration for Text Encoder to handle multiple alphas/ranks, disable/enable adapters and support for multiple adapters #5147

Merged
merged 63 commits into from
Sep 27, 2023
Merged

PEFT Integration for Text Encoder to handle multiple alphas/ranks, disable/enable adapters and support for multiple adapters #5147

merged 63 commits into from
Sep 27, 2023

Conversation

pacman100
Copy link
Contributor

What does this PR do?

  1. support multiple rank/alpha values
  2. support multiple active adapters
  3. support disabling and enabling adapters

younesbelkada and others added 30 commits September 15, 2023 12:58
@younesbelkada
more fixes
ba24f2a
@younesbelkada
up
c17634c
@younesbelkada
up
2a6e535
@younesbelkada
style
01f6d1d
@younesbelkada
add in setup
5a150b2
@younesbelkada
oops
961e776
@younesbelkada
more changes
cdbe739
@younesbelkada
v1 rzfactor CI
691368b
@younesbelkada @patrickvonplaten
Apply suggestions from code review
7918851 
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
@younesbelkada
few todos
14db139
@younesbelkada
Merge branch 'main' into peftpart-1
c06c40b
@younesbelkada
protect torch import
d56a14d
@younesbelkada
style
ec87c19
@younesbelkada
fix fuse text encoder
40a6028
@younesbelkada
Merge remote-tracking branch 'upstream/main' into peftpart-1
0c62ef3
@younesbelkada @sayakpaul
Update src/diffusers/loaders.py
c4295c9 
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
@younesbelkada
replace with recurse_replace_peft_layers
4162ddf
@younesbelkada
keep old modules for BC
1d13f40
@younesbelkada
adjustments on adjust_lora_scale_text_encoder
78a860d
@younesbelkada
Merge branch 'main' into peftpart-1
78a01d5
@younesbelkada
Merge remote-tracking branch 'upstream/main' into peftpart-1
ecbc714
@younesbelkada
Merge branch 'peftpart-1' of https://github.com/younesbelkada/diffusers…
9d650c9 
… into peftpart-1
@younesbelkada
nit
6f1adcd
@younesbelkada
move tests
f890906
@younesbelkada
add conversion utils
f8e87f6
@younesbelkada
Merge remote-tracking branch 'upstream/main' into peftpart-1
3ba2d4e
@younesbelkada
remove unneeded methods
dc83fa0
@younesbelkada
use class method instead
b83fcba
@younesbelkada
oops
74e33a9
@younesbelkada
use base_version
9cb8563
sayakpaul
sayakpaul reviewed Sep 25, 2023
View reviewed changes
sayakpaul
sayakpaul reviewed Sep 25, 2023
View reviewed changes
sayakpaul
sayakpaul reviewed Sep 25, 2023
View reviewed changes
Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Agree with Patrick's and Benjamin's comments. Mine are mostly clarification questions and nits.

pacman100 and others added 4 commits September 26, 2023 16:51
@pacman100 @BenjaminBossan @patrickvonplaten
address comments
1ca4c62 
Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Co-Authored-By: Patrick von Platen <patrick.v.platen@gmail.com>
@pacman100
fix code quality
7c37788
@pacman100
address comments
2fcf174
@pacman100
address comments
a1f0128
sayakpaul
sayakpaul reviewed Sep 27, 2023
View reviewed changes
src/diffusers/loaders.py
@@ -1144,6 +1154,7 @@ def load_lora_weights(self, pretrained_model_name_or_path_or_dict: Union[str, Di
lora_scale=self.lora_scale,
low_cpu_mem_usage=low_cpu_mem_usage,
_pipeline=self,
adapter_name=adapter_name,
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the private variable at the end.

sayakpaul
sayakpaul approved these changes Sep 27, 2023
View reviewed changes
Copy link
Member

@sayakpaul sayakpaul left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks very clean to me! Thanks so much for iterating.

I see only "text_encoder" attribute being used in the PR. Are we not tackling "text_encoder_2" here?

I didn't see any tests, though. Will they be added in a follow-up?

@younesbelkada
Copy link
Contributor

For the tests, I am happy to add them here: #5151 to complete the full testing suite

See my comment: #5151 (comment)

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 27, 2023 •
edited
Loading

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

@patrickvonplaten
Copy link
Contributor

For the tests, I am happy to add them here: #5151 to complete the full testing suite

See my comment: #5151 (comment)

Ok for me to add tests in a later PR

@patrickvonplaten
Copy link
Contributor

Very nice job @pacman100 - think the design is very nice & also agree that higher level enable/disable functions are very useful for the users (to quickly switch on/off all LoRAs)

patrickvonplaten
patrickvonplaten approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Ok for me to merge once CI is green (ok if LoRA peft tests are still failing)

@patrickvonplaten patrickvonplaten merged commit 02247d9 into main Sep 27, 2023
@kashif kashif deleted the smangrul/peft-integration branch September 29, 2023 11:38
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
@pacman100 @younesbelkada
@patrickvonplaten @sayakpaul @BenjaminBossan
PEFT Integration for Text Encoder to handle multiple alphas/ranks, di…
7301c0f 
…sable/enable adapters and support for multiple adapters (huggingface#5147)

* more fixes

* up

* up

* style

* add in setup

* oops

* more changes

* v1 rzfactor CI

* Apply suggestions from code review

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* few todos

* protect torch import

* style

* fix fuse text encoder

* Update src/diffusers/loaders.py

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

* replace with `recurse_replace_peft_layers`

* keep old modules for BC

* adjustments on `adjust_lora_scale_text_encoder`

* nit

* move tests

* add conversion utils

* remove unneeded methods

* use class method instead

* oops

* use `base_version`

* fix examples

* fix CI

* fix weird error with python 3.8

* fix

* better fix

* style

* Apply suggestions from code review

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* Apply suggestions from code review

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* add comment

* Apply suggestions from code review

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

* conv2d support for recurse remove

* added docstrings

* more docstring

* add deprecate

* revert

* try to fix merge conflicts

* peft integration features for text encoder

1. support multiple rank/alpha values
2. support multiple active adapters
3. support disabling and enabling adapters

* fix bug

* fix code quality

* Apply suggestions from code review

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* fix bugs

* Apply suggestions from code review

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* address comments

Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Co-Authored-By: Patrick von Platen <patrick.v.platen@gmail.com>

* fix code quality

* address comments

* address comments

* Apply suggestions from code review

* find and replace

---------

Co-authored-by: younesbelkada <younesbelkada@gmail.com>
Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
@pacman100 @younesbelkada
@patrickvonplaten @sayakpaul @BenjaminBossan
PEFT Integration for Text Encoder to handle multiple alphas/ranks, di…
ce216c4 
…sable/enable adapters and support for multiple adapters (huggingface#5147)

* more fixes

* up

* up

* style

* add in setup

* oops

* more changes

* v1 rzfactor CI

* Apply suggestions from code review

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* few todos

* protect torch import

* style

* fix fuse text encoder

* Update src/diffusers/loaders.py

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

* replace with `recurse_replace_peft_layers`

* keep old modules for BC

* adjustments on `adjust_lora_scale_text_encoder`

* nit

* move tests

* add conversion utils

* remove unneeded methods

* use class method instead

* oops

* use `base_version`

* fix examples

* fix CI

* fix weird error with python 3.8

* fix

* better fix

* style

* Apply suggestions from code review

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* Apply suggestions from code review

Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

* add comment

* Apply suggestions from code review

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

* conv2d support for recurse remove

* added docstrings

* more docstring

* add deprecate

* revert

* try to fix merge conflicts

* peft integration features for text encoder

1. support multiple rank/alpha values
2. support multiple active adapters
3. support disabling and enabling adapters

* fix bug

* fix code quality

* Apply suggestions from code review

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* fix bugs

* Apply suggestions from code review

Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>

* address comments

Co-Authored-By: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Co-Authored-By: Patrick von Platen <patrick.v.platen@gmail.com>

* fix code quality

* address comments

* address comments

* Apply suggestions from code review

* find and replace

---------

Co-authored-by: younesbelkada <younesbelkada@gmail.com>
Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminBossan BenjaminBossan BenjaminBossan left review comments

@sayakpaul sayakpaul sayakpaul approved these changes

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

@younesbelkada younesbelkada younesbelkada approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pacman100 @patrickvonplaten @younesbelkada @HuggingFaceDocBuilderDev @BenjaminBossan @sayakpaul