`from_ckpt` is a bad name

[patrickvonplaten]

Describe the bug

We've gotten a couple of issues like: #3390 that show that from_ckpt is a bad name because people think they cannot load safetensors checkpoints with it. To me ckpt just stood for "checkpoint" and I didn't realize that people strongly associated .ckpt with this method (thinking that any .safetensors checkpoint can't be loaded with from_ckpt)

Let's maybe deprecate the method and rename it no?

Reproduction

Marking this as a bug since multiple people seem to have this problem.

Let's call the new method maybe:

from_single_file(...) or any better ideas fro names cc @sayakpaul @yiyixuxu @pcuenca @williamberman @apolinario ?

Logs

No response

System Info

Not needed

Who can help?

No response

[sayakpaul]

Maybe from_single_ckpt? Checkpoint, to me at least, could mean a single checkpoint file or shards of checkpoints. If we make it abundantly clear from the documentation, will that help?

[JeLuF]

from_single_ckpt would still be strongly associated with ckpt. In order to prevent confusion, it's porbably best to avoid ckpt, or to be very verbose: from_ckpt_or_safetensors

[bghira]

why not just from_local_file or from_single_file?

[sanbuphy]

from_local_file +1
or we can use from_local_checkpoint

[MLator]

Guys, please stop being newbies in software development.

You already made a huge mistake by creating your own very bad format of model checkpoints in multiple folders, and it took you many months to realize that .ckpt is much more popular than HF and was existing before you were born :)

If you don't want a community to give up the Diffusers for a better, clearer, and more stable architecture, then, "No", don't change again the names of functions.
Deprecating is always the worst way to proceed.

If some people ignore what are .ckpt or safetensors, just be very explicit in your explanations and with examples in your documentation (which is almost void, and often seems like a black box...).
https://huggingface.co/docs/diffusers/main/en/api/pipelines/stable_diffusion/img2img#diffusers.StableDiffusionImg2ImgPipeline.from_ckpt

Never forget, that you are not alone, and that when we build some software foundations, on which many people already created millions of lines of code, documentations, videos, tutorials, colabs, tools, etc... we don't change and deprecate the basic elements of building blocks of an API.

Thanks for your understanding !

[bghira]

this is a seldom-used function that is not often used, because it's not well understood.

if we stopped deprecating bad things in favour of better replacements, we would still be on SD 1.4. this is speaking as someone who had great difficulty getting into Huggingface development, because the interfaces changed a lot between versions.

insulting people is also not the way to win hearts and change minds. just because Diffusers does something, doesn't mean you are forced to use it.

[williamberman]

hm interesting, tbh I think something with "local file" is more confusing because it implies something like "this should be used for loading anything locally and doesn't infer anything about the format". Is there an alternative naming convention people use for the .ckpt files that we could put in the name? Something like from_non_diffusers_ckpt or from_ldm_ckpt? I'm pretty open to whatever here

[bghira]

that's a good point. why can't from_pretrained simply be extended to use from_ckpt as if it were an internal method that we don't tell users to rely on?

I remember that being one of the first things I tried was to load a .safetensors or .ckpt file (both were available, and I had no idea what I was doing) via from_pretrained('/path/to/file.safetensors') and programmatically speaking there's no real technical issue preventing that from happening automatically.

[sanbuphy]

that's a good point. why can't from_pretrained simply be extended to use from_ckpt as if it were an internal method that we don't tell users to rely on?

I remember that being one of the first things I tried was to load a .safetensors or .ckpt file (both were available, and I had no idea what I was doing) via from_pretrained('/path/to/file.safetensors') and programmatically speaking there's no real technical issue preventing that from happening automatically.

I agree with you. The first time I attempted to load the safetensors, I simply used the from_pretrained function.

[patrickvonplaten]

Solved with #3968

[MLator]

Oh.... what a genius renaming decision from @patrickvonplaten ! :)
Asking people and then ignoring their answers. The suggestion in the last post from @bghira was simpler.
So as there is now "from_single_file" and "from_pretrained" functions, we can then suppose that a single file is not a pretrained file ?!
And reciprocally, where can we find the function "from_multiple_files" ?

This is really so ridiculous to create massive rework for all users and deprecate functions to rename from this confusing way . Sorry guys, but this is again a bad decision, and someone will have to rename and deprecate again all these functions to put all in order one day. And sorry to tell you truth, but yes you are acting as newbies. I don't care if decision makers don't like it, it's just like seeing a soccer match with newbies scoring against their goal, and I won't tell you that you play as professionals, just to please you.

Have a good evening ! :)

[patrickvonplaten]

There is nothing forbidden about dispatching from_single_files(...) from from_pretrained(...), we can still do this in a future PR. This however requires a lot more work since from_pretrained(...) is the core of all pipelines here. An easier first improvement is to rename from_ckpt here.

Feel free to open a new issue about it @MLator and we'll try to tackle it (or give it a try yourself if you'd like to show us newbies how it's done haha)