-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
failed to use the feature of supporting for A1111 LoRA #3725
Comments
cc @sayakpaul |
Hi @icech. Could you share your LoRA file so that we can debug it on our end? Cc: @takuma104 |
The link of my LoRA file is |
@icech I am able to use the LoRA without any problems. See my Colab: Of course, I don't know about the base pipeline associated with that. So, you will have to work that one out. |
I'm sorry for the delayed response. I have reviewed your Colab notebook and made some modifications. I can provide a comparison between the version with and without Lora, and generate four images. Only one of the images will be different.
|
I see what you're saying. Reproduced here: https://colab.research.google.com/gist/sayakpaul/b645715d9144a3a6dc40c93bdceee929/scratchpad.ipynb. Some questions:
|
That's quite an interesting result. It seems that the effect varies in magnitude, but it's not just the first one that differs; there appears to be some change in all of them. I've posted an image created by merging the two results using the difference mode in Photoshop. |
I've seen the graph in your Colab. they are the same as what I drew before. I'm sorry that I can't provide any pictures these days since I'm on vacation and don't have access to my machine. However, you can test the 'load_lora_weights(pipeline, checkpoint_path, multiplier, device, dtype)' I originally mentioned to replace 'pipeline.load_lora_weights' for loading lora. The usage is 'pipe = load_lora_weights(pipe, lora_path, 1.0, 'cuda', torch.float32)', so you can make a comparison. Based on my previous experience, the two should not be consistent, and the version of @pdoane is the expected result, consistent with A111. |
Your analysis is very rigorous. Indeed, there are differences in the subsequent images, but these differences are not as expected. |
I will dive more to find out what we're missing :) But expect some delay as I am on the move and away for sometime. |
@icech use this: thank me later. |
I've tried this and I know it's feasible, but I mainly want to use the official API for easier maintenance in the future. Thank you anyway. |
Let's try to fix this this week so it's in the next release cc @sayakpaul , could this maybe be fixed with: #3778 ? |
@icech I am trying to understand this better and would appreciate your inputs here. If I do Regardless, I will dive deeper into the loaded parameters and see what we're missing out on :) |
This is same as my experiments. I will provide some images of my result tomorrow(about 10 hours later) for you to compare. |
Went deep into this issue. TL;DR: With the current support for loading A1111 in Diffusers, we are unable to load certain keys, especially the ones containing I was able to use @pdoane's script and generate the expected outputs. Check out this Colab. You'd notice that their method allows for merging all the weights where the current diffusers support doesn't allow that. We cannot go the merging way in diffusers as it doesn't allow for switching to a later attention processor easily. This is the primary reason. With #3756, this should be addressed and hopefully resolved. To make this finding even more concrete, I prepared this script: https://gist.github.com/sayakpaul/c269da54270f6d866ef5acafd4bf8319. This shows us that, indeed, we're not loading all the keys and it's actually a known phenomenon. Thanks for bringing this to our attention. And hopefully, we should be able to fix this soon. |
I was interested in trying to add loras to diffusers and stumbled upoun this code snippet. It works for some loras but not all. So I took some new code from AUTOMATIC1111/stable-diffusion-webui#11821 which implements more lora support and converted it to work like the snippet above. So it merges the layers into pipeline as opposed to hooking onto torch forward functions. The code can be found here: https://github.com/CapsAdmin/diffusers-a1111/blob/main/src/merge_lora_to_pipeline.py I tested all the models mentioned in the a1111 pr, so it supports hada, ia3, lokr, "full" and lora. The script is self contained apart from importing "shared" which is just some dtype and device variables. So perhaps this is of interest to you @sayakpaul for testing on collab or something. |
@CapsAdmin thanks so much! Does that script work for SDXL LoRA checkpoints too? Maybe it would make sense if you created a converter space with your script to let people easily use it (like this one: https://huggingface.co/spaces/diffusers/sd-to-diffusers)?
Unfortunately, by our design, we're a bit hesitant to directly merge the weights into the concerned modules. So, we will have to think about it a bit. |
I can try to get SDXL working, there was a very small amount of additional code that supposedly enabled it but I left it out to focus on getting it working with 1.5.
It was just intended as something you'd plug into someone's diffusers backend, ie sdnext (the a1111 fork) is currently moving to diffusers but it currently does not support loading lora's the way the original backend does. Since I'm merging this into the pipeline, I guess this is not far from a "merge loras into diffusers checkpoint" utility, but I don't really see the necessity for something like that. The other use case I intended for this is just that it could be an example/debug implementation for diffusers to do it properly.
When it comes to merging into the pipeline, I see pros and cons and I'm honestly not sure which is better. Keep in mind I'm not very versed in this space. pros:
cons:
If you wanted to support loading and unloading on the fly there are ways to merge internally by keeping track of the changes a lora does to a pipeline, but this is very messy. Maybe you could even unload a lora by reversing the calculation, however with this method I would worry about losing precision. |
Hey @icech could you give #4147 a try? Just install Here I have hosted a couple of samples for you https://huggingface.co/datasets/sayakpaul/3725_test/. Here's a side-by-side comparison:
Let us know your findings! Cc: @isidentical. |
@CapsAdmin also thanks for explaining this. We're trying to improve the support in #4147 thanks to @isidentical. Watch out :) |
I am glad to see the diffusers add the supporting for A1111 LoRA. However, i failed to ues this feature after I update the diffusers.
It did not exporting error as before, but the lora have no effect in the generated images.
I use it as following:
and this is how I used to add lora in the past which is form #3064 by @pdoane :
Is it my incorrect usage or is there a difference between the current code and what @pdoane provided?
The text was updated successfully, but these errors were encountered: