Skip to content

[lora] adapt new LoRA config injection method - #11999

Merged
sayakpaul merged 18 commits into
mainfrom
exclude-modules-peft
Aug 8, 2025
Merged

[lora] adapt new LoRA config injection method#11999
sayakpaul merged 18 commits into
mainfrom
exclude-modules-peft

Conversation

@sayakpaul

@sayakpaulsayakpaul commented Jul 28, 2025•
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Fixes#11874

Relies on huggingface/peft#2637

Supercedes #11911

TODO: Run some integration tests before merging.

Edit: Have run the integration tests for SDXL and Flux and they work.

sayakpaul
sayakpaul commented Jul 28, 2025
View reviewed changes
Comment threadsetup.py Outdated
"numpy",
"parameterized",
"peft>=0.15.0",
# "peft>=0.16.1",

@sayakpaulsayakpaulJul 28, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

To be changed when peft has the release.

This was referenced Jul 28, 2025
Closed
Merged
@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Jul 28, 2025

Copy link
Copy Markdown

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.

@sayakpaul

sayakpaul commented Aug 2, 2025

Copy link
Copy Markdown
MemberAuthor

Now that PEFT 0.17.0 is out, we should be good to review this PR. Gentle ping @BenjaminBossan

BenjaminBossan
BenjaminBossan reviewed Aug 4, 2025
View reviewed changes
Comment threadtests/models/transformers/test_models_transformer_flux.py
@sayakpaul
sayakpaul marked this pull request as ready for review August 4, 2025 11:26
BenjaminBossan
BenjaminBossan approved these changes Aug 6, 2025
View reviewed changes

@BenjaminBossanBenjaminBossan 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.

Looks good from my side, thanks.

@sayakpaul
sayakpaul requested a review from DN6August 6, 2025 15:07
@sayakpaul

sayakpaul commented Aug 6, 2025

Copy link
Copy Markdown
MemberAuthor

@DN6 could you give this a check please? This solves some existing issues around complicated logic of deriving some LoraConfig parameters.

@sayakpaulsayakpaul mentioned this pull request Aug 8, 2025
Closed
DN6
DN6 approved these changes Aug 8, 2025
View reviewed changes

@DN6DN6 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.

LGTM 👍🏽 Just need tests to pass.

@sayakpaul
sayakpaul merged commit a8e4797 into mainAug 8, 2025
31 of 32 checks passed
@sayakpaul
sayakpaul deleted the exclude-modules-peft branch August 8, 2025 03:52
@sayakpaul

sayakpaul commented Aug 8, 2025

Copy link
Copy Markdown
MemberAuthor

Failing test is unrelated.

@christopher5106

christopher5106 commented Aug 8, 2025

Copy link
Copy Markdown
Contributor

Still not 100% layers loaded

#12053 (comment)

@burgalon

burgalon commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

inference with lora suddenly significantly slower than with diffusers==0.33.1 and peft==0.14.0
Used to be 3.9it/s now 2.9it/s

@a-r-r-o-wa-r-r-o-w mentioned this pull request Aug 12, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BenjaminBossanBenjaminBossanBenjaminBossan approved these changes
@DN6DN6DN6 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.

[lora] exclude_modules won't consider the modules names in target_modules

6 participants

@sayakpaul@HuggingFaceDocBuilderDev@christopher5106@burgalon@BenjaminBossan@DN6