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

Some Flux loras can't be applied #370

Open
stduhpf opened this issue Aug 26, 2024 · 12 comments
Open

Some Flux loras can't be applied #370

stduhpf opened this issue Aug 26, 2024 · 12 comments

Comments

@stduhpf
Copy link
Contributor

stduhpf commented Aug 26, 2024

Some Flux loras are working, but others refuse to load properly (even though they work fine on comfyui). Around 50 % of the loras I've tested where failing.

There is no crash, but I get lots of [WARN ] lora.hpp:176 - unused lora tensor transformer.xxx, and it ends with [WARN ] lora.hpp:186 - Only (0 / N) LoRA tensors have been applied.

I'm not 100% sure what's going on, but a pattern I think I've noticed is that the Loras that can load all have 912 tensors, while all the ones who fail all have some different number of tensors?

Example of lora-related logs
[WARN ] stable-diffusion.cpp:617  - In quantized models when applying LoRA, the images have poor quality.
[INFO ] stable-diffusion.cpp:635  - Attempting to apply 1 LoRAs
[INFO ] model.cpp:789  - load ..\ComfyUI\models\loras\ana_de_armas_flux_lora_v1_000002000.safetensors using safetensors format
[DEBUG] model.cpp:857  - init from '..\ComfyUI\models\loras\ana_de_armas_flux_lora_v1_000002000.safetensors'
[INFO ] lora.hpp:33   - loading LoRA from '..\ComfyUI\models\loras\ana_de_armas_flux_lora_v1_000002000.safetensors'
[DEBUG] model.cpp:1526 - loading tensors from ..\ComfyUI\models\loras\ana_de_armas_flux_lora_v1_000002000.safetensors
[DEBUG] ggml_extend.hpp:1029 - lora params backend buffer size =  327.75 MB(RAM) (988 tensors)
[DEBUG] model.cpp:1526 - loading tensors from ..\ComfyUI\models\loras\ana_de_armas_flux_lora_v1_000002000.safetensors
[DEBUG] lora.hpp:69   - finished loaded lora
[WARN ] lora.hpp:176  - unused lora tensor transformer.single_transformer_blocks.0.attn.to_k.lora_A.weight
[WARN ] lora.hpp:176  - unused lora tensor transformer.single_transformer_blocks.0.attn.to_k.lora_B.weight
[...] (This goes on for all the tensors in the model)
[WARN ] lora.hpp:176  - unused lora tensor transformer.transformer_blocks.9.norm1_context.linear.lora_A.weight
[WARN ] lora.hpp:176  - unused lora tensor transformer.transformer_blocks.9.norm1_context.linear.lora_B.weight
[WARN ] lora.hpp:186  - Only (0 / 988) LoRA tensors have been applied
@Green-Sky
Copy link
Contributor

Green-Sky commented Aug 26, 2024

@stduhpf please dont copy reply to likely malicious binaries!

@stduhpf
Copy link
Contributor Author

stduhpf commented Aug 26, 2024

@stduhpf please dont copy reply to likely malicious binaries!

Oops, edited out

@grauho
Copy link
Contributor

grauho commented Aug 26, 2024

Based on the log you posted my initial thought is that some of the LoRAs you're using have a non-standard naming convention so when they looks for the corresponding tensors in the model it can't find them. Might be the fact that they're using a *_{A,B} convention instead of *_{up,down} but that's just a hunch at the moment.

@stduhpf
Copy link
Contributor Author

stduhpf commented Aug 26, 2024

Hmm could be... I'll try replacing "up" and "down" in the code with "A" and "B" and see how it goes.

@grauho
Copy link
Contributor

grauho commented Aug 26, 2024

Hmm could be... I'll try replacing "up" and "down" in the code with "A" and "B" and see how it goes.

Another option is just to run a quick grep on the LoRAs that do load without an issue for that pattern and then on the ones that don't and see if it's a straight one to one correlation

Because of how the name preprocessing works I'm not sure that just replacing the up / down bit in model.cpp will be enough unless the convention in the non-functional LoRAs happens to be identical to a supported convention except for that one suffix.

@stduhpf
Copy link
Contributor Author

stduhpf commented Aug 26, 2024

Hmm could be... I'll try replacing "up" and "down" in the code with "A" and "B" and see how it goes.

Another option is just to run a quick grep on the LoRAs that do load without an issue for that pattern and then on the ones that don't and see if it's a straight one to one correlation

Because of how the name preprocessing works I'm not sure that just replacing the up / down bit in model.cpp will be enough unless the convention in the non-functional LoRAs happens to be identical to a supported convention except for that one suffix.

Yep, this doesn't seem to work.

@grauho
Copy link
Contributor

grauho commented Aug 26, 2024

Hmm could be... I'll try replacing "up" and "down" in the code with "A" and "B" and see how it goes.

Another option is just to run a quick grep on the LoRAs that do load without an issue for that pattern and then on the ones that don't and see if it's a straight one to one correlation
Because of how the name preprocessing works I'm not sure that just replacing the up / down bit in model.cpp will be enough unless the convention in the non-functional LoRAs happens to be identical to a supported convention except for that one suffix.

Yep, this doesn't seem to work.

And is it the case where all the non-loading LoRAs have this *_{A,B} pattern while all the working ones do not?

@grauho
Copy link
Contributor

grauho commented Aug 26, 2024

To test this try using:
grep -l "lora_[AB]" <FILES>

on the LoRAs you used for the initial run and see if the names printed correspond to the ones that didn't load. Alternatively you could run the following to try to match the files that did load:
grep -E -l 'lora_up|lora_down' <FILES>

@stduhpf
Copy link
Contributor Author

stduhpf commented Aug 26, 2024

To test this try using: grep -l "lora_[AB]" <FILES>

on the LoRAs you used for the initial run and see if the names printed correspond to the ones that didn't load. Alternatively you could run the following to try to match the files that did load: grep -E -l 'lora_up|lora_down' <FILES>

Yes it matches perfectly. All those who match "lora_up|lora_down" work, and all the others match "lora_[AB]"

@stduhpf
Copy link
Contributor Author

stduhpf commented Aug 26, 2024

https://github.com/comfyanonymous/ComfyUI/blob/master/comfy/lora.py#L53 Maybe we could find a solution by looking at this implementation.

@grauho
Copy link
Contributor

grauho commented Aug 26, 2024

Yeah it looks like perhaps some of the new flux LoRA training scripts have decided to use a different variant of the diffusers naming convention, probably won't be too bad to fix and will just require another conversion function during the tensor name pre-processing like with the SDXL conversion.

Edit: Might be more complex than it seemed at first blush as it seems like the flux model does some strange bundling of the q, k, and v attention heads while the LoRA handles them independently. The rest will hopefully be a simple mapping.

@github-staff github-staff deleted a comment from stduhpf Aug 27, 2024
@taotaow
Copy link

taotaow commented Aug 30, 2024

https://huggingface.co/ByteDance/Hyper-SD/blob/main/Hyper-SD3-8steps-CFG-lora.safetensors can't apply
It's tensor name are all like transformer.*.lora_A/B.weight

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

No branches or pull requests

7 participants
@Green-Sky @stduhpf @taotaow @grauho and others