-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[LoRA] Fix SDXL text encoder LoRAs #4371
Conversation
if is_network_alphas_populated and len(network_alphas) > 0: | ||
raise ValueError( | ||
f"The `network_alphas` has to be empty at this point but has the following keys \n\n {', '.join(network_alphas.keys())}" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we show ValueError as warning log (non supported lora ) and move on instead of raise error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a ValueError is great here
The documentation is not available anymore as the PR was closed or merged. |
Lycoris still not working for SDXL https://civitai.com/models/108011/fcbodybuildingxl-10-for-sdxl |
I think we talked about it in the other thread too. It might be happening because LyCORIS adapters can contain LoHA modules too which we don't support yet. Let's try not to duplicate these things :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Only left a couple of minor suggestions
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
Merging a potentially impactful one. Fingers crossed! |
* temporarily disable text encoder loras. * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debbuging. * modify doc. * rename tests. * print slices. * fix: assertions * Apply suggestions from code review Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com> --------- Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
* temporarily disable text encoder loras. * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debugging * debbuging. * modify doc. * rename tests. * print slices. * fix: assertions * Apply suggestions from code review Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com> --------- Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
This PR fixes the LoRA loading part for the text encoders, especially the ones we see for SDXL 0.9.
Additionally, it also refactors some of the loading logic to help make the
state_dict
parsing easier.I tested with the following:
And got:
LoRA is from here: https://civitai.com/models/108448/daiton-sdxl-test
@patrickvonplaten I see we don't do SLOW tests for SDXL, probably because we run the SLOW tests on a V100, which might fall short of running SDXL. I am bringing it up because I wanted to add a couple of slow tests to
test_lora_layers.py
to test the SDXL LoRAs from CivitAI.Some comments are in line.
Also, I have run the SLOW tests and they pass successfully.