Skip to content

Fix regression introduced in #2448 - #2551

Merged
patrickvonplaten merged 2 commits into
huggingface:mainfrom
Narsil:main
Mar 4, 2023
Merged

Fix regression introduced in #2448#2551
patrickvonplaten merged 2 commits into
huggingface:mainfrom
Narsil:main

Conversation

@Narsil

@Narsil Narsil commented Mar 4, 2023

Copy link
Copy Markdown
Contributor

Fix #2448
This will try to load safetensors from filename, only if the extension
matches .safetensors.

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Mar 4, 2023

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@gadicc

gadicc commented Mar 4, 2023

Copy link
Copy Markdown

Thanks, @Narsil, for your crazy fast work here. Greatly appreciated.

I just have one comment. This indeed fixes the regression, and I think checking the weights filename for safetensors is a very sensible default, but it would still be nice to be able to override this. Maybe with an arg like from_safetensors which I've seen used in other parts of diffusers? What do you think?

@gadicc

gadicc commented Mar 4, 2023

Copy link
Copy Markdown

Oh, and sorry, another comment 😅

Since pretrained_model_name_or_path_or_dict can be the LoRA file too, we'd need to do the endswith(".safetensors") check on that too I think?

@pcuenca pcuenca 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, thanks a lot for the test @Narsil, it was very helpful as a repro (and to prevent future regressions, of course) :)

@patrickvonplaten
patrickvonplaten merged commit 2ea1da8 into huggingface:main Mar 4, 2023
@Narsil

Narsil commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

. Maybe with an arg like from_safetensors which I've seen used in other parts of diffusers? What do you think?

It has some merit, but I feel the complexity of this function has already gone through the roof, so I would avoid changing it without taking the opportunity for refactor.

Maybe load_attn_safetensors(filename) which could get called by the current function (and incidently the symetric for torch version).

But that's more than the PR should be responsible for. I'll let core maintainers decide on the best option.

@gadicc

gadicc commented Mar 6, 2023

Copy link
Copy Markdown

Hey, @Narsil. Sounds good. Thanks so much 🙏

@stevensu1977

stevensu1977 commented Mar 6, 2023

Copy link
Copy Markdown

Hi , @Narsil I use your loaders.py code , try to load LoRA use safetensors format(https://huggingface.co/prompthero/openjourney-lora/tree/main) ,
but not luckly, get erorr, did I missing something ?

File "model_test.py", line 52, in
pipe.unet.load_attn_procs("./openjourneyLora.safetensors")
File "/usr/local/lib/python3.8/dist-packages/diffusers/loaders.py", line 207, in load_attn_procs
rank = value_dict["to_k_lora.down.weight"].shape[0]
KeyError: 'to_k_lora.down.weight'

@Narsil

Narsil commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

Do you have the original weights ?
From the looks of it, it just seems that that tensor is missing from the lora weights you're using. But I'm not sure.

@Meander112381442

Copy link
Copy Markdown

I've gotten the same error trying to load .safetensors loras from https://civitai.com using the updated loaders.py.
(e.g. trying to load https://civitai.com/models/8730/hipoly-3d-model-lora)

KeyError Traceback (most recent call last)
/var/folders/m8/b3gg5_fj303cdk329ysphvd80000gp/T/ipykernel_40499/1445337579.py in
----> 1 pipeImg2Img.unet.load_attn_procs("path/LORA/hipoly3DModelLora_v10.safetensors")

~/opt/anaconda3/lib/python3.9/site-packages/diffusers/loaders.py in load_attn_procs(self, pretrained_model_name_or_path_or_dict, **kwargs)
204
205 for key, value_dict in lora_grouped_dict.items():
--> 206 rank = value_dict["to_k_lora.down.weight"].shape[0]
207 cross_attention_dim = value_dict["to_k_lora.down.weight"].shape[1]
208 hidden_size = value_dict["to_k_lora.up.weight"].shape[0]

KeyError: 'to_k_lora.down.weight'

@Narsil

Narsil commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

Are those supposed to work with diffusers ?

@Meander112381442

Copy link
Copy Markdown

I've gotten satisfactory results with a model hosted on https://civitai.com/ and assumed that they could be adjusted by any kind of lora. I only have a vague/superficial idea of the inner workings of this algorithm, so feel free disregard my report if you deem it nonsensical.

@Narsil

Narsil commented Mar 7, 2023

Copy link
Copy Markdown
Contributor Author

@Meander112381442 I don't know either, but I think civitai is using mostly weights in SD format, which just have different names and layouts. So technically you should be able to use them, but I'm not sure where to point you, but probably some stuff has to be modified either here in diffusers or in the lora file.

@sonaterai

Copy link
Copy Markdown

Hello, Any idea why i got this error too ?
KeyError: 'to_k_lora.down.weight'

@Narsil

Narsil commented Mar 9, 2023

Copy link
Copy Markdown
Contributor Author

I am guessing it's a file meant for SD and not diffusers, Sorry I don't know if and where could conversion script be.

@sonaterai

Copy link
Copy Markdown

I understand, if someone has a script that will convert a lora safetensor to diffuser I'm really interested
thanks very much

@DaemonAlchemist

Copy link
Copy Markdown

I second that. It would be nice if the diffusers library could load files in either format.

@wrt2000

wrt2000 commented Mar 24, 2023

Copy link
Copy Markdown

Same problem:
KeyError: 'to_k_lora.down.weight'

mengfei25 pushed a commit to mengfei25/diffusers that referenced this pull request Mar 27, 2023
@Pirog17000

Copy link
Copy Markdown

same problem KeyError: 'to_k_lora.down.weight'

@patrickvonplaten

Copy link
Copy Markdown
Contributor

Hey - thanks for the ping everybody! Could we maybe open a new issue for #2551 (comment) - this is not what was fixed in this PR. If I understand correctly the request here is to be able to load checkpoints trained with: https://github.com/cloneofsimo/lora into diffusers which is not really what the PR was about.

This will require some more changes to diffusers - would be great if we could move this to a new issue (also cc @sayakpaul )

w4ffl35 pushed a commit to w4ffl35/diffusers that referenced this pull request Apr 14, 2023
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
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

Successfully merging this pull request may close these issues.