Improve SD35 LoRA support to cover most popular LoRA formats

[vladmandic]

SD3.x pipeline does implement SD3LoraLoaderMixin and as such load_lora_weights on SD3.x does "work".

However, attempting to load any of the most popular LoRAs results in silent failure:
load is successful without any warnings, but loads ZERO keys.

Looking at implementation at:

diffusers/src/diffusers/loaders/lora_pipeline.py

Line 1214 in 345907f

def load_lora_weights(

Shows that if there are no matching keys in text_encoder_state_dict, text_encoder_state_dict, text_encoder_2_state_dict, there is no warning raised, the method simply does NOTHING.

When looking at state_dict of loaded LoRA, it shows that keys are NOT in the expected format - they need remapping.
For example:
dict_keys(['lora_unet_joint_blocks_0_context_block_adaLN_modulation_1.alpha', 'lora_unet_joint_blocks_0_context_block_adaLN_modulation_1.lora_down.weight', 'lora_unet_joint_blocks_0_context_block_adaLN_modulation_1.lora_up.weight', 'lora_unet_joint_blocks_0_context_block_attn_proj.alpha' ...])

Below are download links for some of the popular LoRAs for SD35, all experience same behavior:

• https://civitai.com/models/943882/tokino-sd35-ambient-scene?modelVersionId=1056722
• https://civitai.com/models/539031/lineart-sd35fluxsdxlpony?modelVersionId=1052194
• https://civitai.com/models/948259/brian-sum-mech-lora-sd35-large?modelVersionId=1061663

CC @yiyixuxu @sayakpaul @DN6 @asomoza
CC @AI-Casanova

[sayakpaul]

Ah I know why this is happening.

It's because when we supported SD3 LoRA, there weren't popular non-diffusers LoRAs. But clearly, things have changed now. I will look into it.

[bghira]

could you please ensure that the missing keys error out in a more generalised sense @sayakpaul ? that way this might not impact newer models when implementing their LoRA loaders

[sayakpaul]

could you please ensure that the missing keys error out in a more generalised sense

I don't understand what that means.

[bghira]

this happened for Flux and SD3 now and I would rather that missing keys never go unnoticed?

[sayakpaul]

We cannot predict beforehand what should be the preferred structure to detect those missing keys. It's still a bit ad-hoc in nature. But as soon as we do, we try our best to make these robust. So, that will very likely be the case.

A bit different but similar in theme is this PR: #9535. As soon as this was reported, we fixed it with a test case.

[vladmandic]

i agree with @bghira, the issue is silent nature of failures.
@sayakpaul, imagine a scenario where lora has 80% of keys matched and 20% simply ignored - who would ever report that? users would not even know there is something wrong, lora would work, but look just a bit off in the outputs.
why not print a warning statement if there are unmatched keys (and how many)? it doesn't need to be more verbose than that as if it is a non-zero value user will at least know to look deeper.

[sayakpaul]

imagine a scenario where lora has 80% of keys matched and 20% simply ignored - who would ever report that? users would not even know there is something wrong, lora would work, but look just a bit off in the outputs.
why not print a warning statement if there are unmatched keys (and how many)? it doesn't need to be more verbose than that as if it is a non-zero value user will at least know to look deeper.

1. We recently shipped support for that already. See: https://github.com/huggingface/diffusers/blob/7ac6e286ee994270e737b70c904ea50049d53567/src/diffusers/loaders/peft.py#L257C13-L280C41
2. We try to make sure that the state dict conversion of a non-diffusers LoRA checkpoint is as expected. Such as:

diffusers/src/diffusers/loaders/lora_conversion_utils.py

Line 123 in 7ac6e28

raise ValueError("At this point all state dict entries have to be converted.")

diffusers/src/diffusers/loaders/lora_conversion_utils.py

Line 553 in 7ac6e28

logger.warning(f"Unsupported keys for ai-toolkit: {sds_sd.keys()}")

To better track this issue (I plan to work on this one very soon treating it as a high-prio). For any further discussions, a separate issue thread would be better, IMO.

[bghira]

it looks like that commit covers the behaviour i expected so i don't think a new issue is still needed? just a new diffusers release would be great for most users i think 😉

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.