-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Allow non-default schedulers to be easily swapped into DiffusionPipeline
classes
#183
Comments
Anybody keen on taking this one? Should be rather easy |
I like this solution, much cleaner! But I have one concern, this solution doesn't really swap the scheduler if I understand it correctly. It rather initilizes the whole pipeline again, which is a bit slow. By swapping I would expect to just swap the scheduler while keeping everything else in-place. Should we maybe add some method called |
@patil-suraj nono it shouldn't init the pipeline again - if implemented correctly there is no need to re-initialize anything. |
Thanks for clarifying, if we don't need to re-initialize anything then let's go with it ! |
I think @patil-suraj might be referring to a specialized use case where someone would want to experiment with different schedulers in the same session. So they'd create a pipeline with the default scheduler, and then they'd need to create a new pipeline with a different scheduler. Is this something worthwhile doing, especially given the can of worms @patrickvonplaten mentioned? |
good morning friends, sorryx my ignorance, but I am getting an error in the HF pipeline route and I honestly don't know what commands I should write in colab, thank you very much. |
Hi @Jpbonino , could you open a different issue and post the code-snippet and the error that you are having ? |
done @patil-suraj #193 |
Issue has been solved with #188 |
Maybe I'm misunderstanding, but I don't understand how #188 helps 'swap' a scheduler in an existing pipeline. But my reading above is that the code in the original post:
is OK and acceptable to change a scheduler on an existing pipeline? The reason swapping is desirable is if you want to support X different schedulers, if you can't swap, you need to load X different pipelines, which means you need X times more memory, or break it apart into multiple processes/sessions. |
No, you have to set it in the |
Hey @reimager, We advise to use the following API to swap schedulers: from huggingface_hub import login
from diffusers import DiffusionPipeline
import torch
# Now we can download the pipeline
pipeline = DiffusionPipeline.from_pretrained("runwayml/stable-diffusion-v1-5", torch_dtype=torch.float16)
from diffusers import DDIMScheduler
pipeline.scheduler = DDIMScheduler.from_config(pipeline.scheduler.config) Note that a longer tutorial is also here: https://huggingface.co/docs/diffusers/using-diffusers/schedulers |
Is the PNDM scheduler still the default scheduler for SD? If so, why? According to the Huggingface article Training Stable Diffusion with Dreambooth using 🧨 Diffusers, DDIM usually works much better than PNDM and LMSDiscrete for overfitted models. So, from an end user perspective, what are the advantages of PNDM? |
@jslegers That article talks about different schedulers during inference. Now people are already using something like DPM++ SDE @ 20 steps. It's not the noise scheduler during training. I tested PNDM and DDIM for dreambooth training, and they both work the same. |
Thing is, I've been working on my own finetuned general purpose SD 1..5 model, similar to Openjourney Diffusion, Dreamlike Diffusion & Seek.art_META. In my experience, the PNDM scheduler is pretty bad for running this type of models in the DiffusionPipeline, as the DDIM Scheduler gives much better results (at least for 20 steps)... which is why it puzzles me why the PNDM scheduler is the default. Thusfar, as an end user, I have no clue why I'd ever want to use the PNDM scheduler on any model and why I shouldn't make the DDIM or DPM++ Scheduler the default scheduler for all my finetuned models... As a software developer who's still fairly new to AI, I find any documentation I can find on schedulers pretty cryptic and inaccessible... |
@jslegers You're just confused between noise scheduler during training and noise scheduler during inference. The article you mentioned clearly states "Using DDIM for inference seems to be more robust. " Noise schedulers for training don't seem to matter at all, because you're setting a discrete learning rate anyway. If you switch PNDM to other schedulers, you will just overfit the model. |
I'm not confused. I am indeed refering to inference. Why do you presume I'm talking about training? When you load a 1.x model into a DiffusionPipeline, it loads PNDM scheduler by default for inference, as described in the config.json. I don't understand why this is the default if this is an inferior scheduler for inference. It took me countless hours of tinkering & a bit of luck to figure out I could greatly improve he performance & quality of my models (or any other model, really) during inference just by changing the scheduler in the json config. |
@jslegers Told you even DDIM in that article is outdated info; nobody uses it right now for SD inferences. You're supposed to replace the default with one of the latest noise schedulers, which are Euler Ancestral, PLMS, and DPM++ SDE. You also need to set your own step count that works best for the noise scheduler. There's nothing wrong with PNDM. It's just slow and also requires more step count. |
I find that out yesterday... after wasting dozens of hours trying out different models by running them in diffusers with a PNDM scheduler. I'm getting good results with DPM++ and changed the default for all my models to DPM++. Would you say that is the best to use as a default. If not, which would you recommend? I know the AUTOMATIC1111 GUI used to use Euler a as the default, but it's ages ago since I used it... Pretty much every other SD diffusers model on Huggingface still uses PNDM, though. Same for scripts that convert ckpt to diffusers format. It's all PNDM... except for Seek.art_META & Dreamlike Diffusion, which use DDIM... I suppose most devs either can't keep up with recent developments or simply don't care about the diffusers model because most end users use the ckpt file to run it on AUTOMATIC1111. Either way, as someone who's struggling to find his way through the maze that is the SD ecosystem it's pretty damn frustrating to find out dozens of hours were wasted due to outdated settings in config files... |
All the torch_models are imported to gs::shark_tank. Scripts have been updated.
Is your feature request related to a problem? Please describe.
By default the stable diffusion pipeline uses the PNDM scheduler, but one could easily use other schedulers (we only need to overwrite the
self.scheduler
) attribute.This can be done with the following code-snippet:
Now, that's a bit hacky and not the way we want users to do it ideally!
Describe the solution you'd like
Instead, the following code snippet should work or less for all pipelines:
This is a cleaner & more intuitive API. The idea should be that every class variable that can be passed to
diffusers/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion.py
Line 15 in 051b346
from_pretrained(...)
When currently running this command it fails:
Now we can allow such behavior by adding some logic to the general DiffusionPipeline
from_pretrained
method here:diffusers/src/diffusers/pipeline_utils.py
Line 115 in 051b346
Also we want this approach to work not just for one pipeline and only the scheduler class, but for all pipelines and all schedulers classes.
We can achieve this by doing more or less the following in
diffusers/src/diffusers/pipeline_utils.py
Line 115 in 051b346
Pseudo code:
diffusers/src/diffusers/pipeline_utils.py
Line 149 in 051b346
-> you should get a list of keys such as
[vae, text_encoder, tokenizer, unet, scheduler]
kwargs
-> if yes -> store them in a dictpassed_class_obj
diffusers/src/diffusers/pipeline_utils.py
Line 162 in 051b346
name
is inpassed_class_obj
dict -> if yes -> simple use this instead and skip the loading part (set the passed class toloaded_sub_model
)=> after the PR this should work:
where as this should give a nice error message (note how scheduler is incorrectly passed to vae):
The error message can be based on the passed class not having a matching parent class with what was expected (this could be checked using this dict:
diffusers/src/diffusers/pipeline_utils.py
Line 34 in 051b346
Additional context
As suggusted by @apolinario - it's very important to allow one to easily swap out schedulers. At the same time we don't want to create too much costum code. IMO the solution above handles the problem nicely.
The text was updated successfully, but these errors were encountered: