-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[Better scheduler docs] Improve usage examples of schedulers #890
Conversation
The documentation is not available anymore as the PR was closed or merged. |
@@ -72,7 +72,7 @@ image.save("astronaut_rides_horse.png") | |||
# make sure you're logged in with `huggingface-cli login` | |||
from diffusers import StableDiffusionPipeline, DDIMScheduler | |||
|
|||
scheduler = DDIMScheduler(beta_start=0.00085, beta_end=0.012, beta_schedule="scaled_linear", clip_sample=False, set_alpha_to_one=False) | |||
scheduler = DDIMScheduler.from_config("CompVis/stable-diffusion-v1-4", subfolder="scheduler") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scheduler_config does not specify values for clip_sample
or set_alpha_to_one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Wonder if we could somehow do this automatically - maybe just add those values there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually opening a bigger issue for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch! Yeah I noticed it's acutally not that easy - we'll need some more code changes here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be correct now by simply setting clip_sample
in the original configuration file - even if it's not expected by the scheduler => the original PNDMScheduler config should therefore just have clip_sample
set to False
for DDIM. This logic is now implemented via the class attribute _compatible_classes
.
@@ -72,6 +72,19 @@ def __init__( | |||
new_config["steps_offset"] = 1 | |||
scheduler._internal_dict = FrozenDict(new_config) | |||
|
|||
if hasattr(scheduler.config, "clip_sample") and scheduler.config.clip_sample is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As soon as the 0.7.0 is out, we will add clip_sample
to https://huggingface.co/runwayml/stable-diffusion-v1-5 so that the default scheduler config is 1-to-1 compatible with all other schedulers
dcf19eb
to
38296c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very clean and cool, thanks a lot for working on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, and extremely useful! I just left some nits.
src/diffusers/pipelines/stable_diffusion/pipeline_onnx_stable_diffusion.py
Outdated
Show resolved
Hide resolved
src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_inpaint_legacy.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Pedro Cuenca <[email protected]>
…better_docs_schedulers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
noise_scheduler = DDPMScheduler( | ||
beta_start=0.00085, beta_end=0.012, beta_schedule="scaled_linear", num_train_timesteps=1000 | ||
) | ||
noise_scheduler = DDPMScheduler.from_config("CompVis/stable-diffusion-v1-4", subfolder="scheduler") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is both a config value and conflicts with --pretrained_model_name_or_path
, this was previously compatible with local and remote sd1.4 and sd1.5 in dreambooth, text_to_image, and textual inversion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @kjerk , would you like to open a PR to fix this ?
…face#890) * [Better scheduler docs] Improve usage examples of schedulers * finish * fix warnings and add test * finish * more replacements * adapt fast tests hf token * correct more * Apply suggestions from code review Co-authored-by: Pedro Cuenca <[email protected]> * Integrate compatibility with euler Co-authored-by: Pedro Cuenca <[email protected]>
Let's try to advocate as much as possible to load schedulers directly from the saved configs instead of having to define the parameters manually.
I.e. we don't want to see any:
but instead:
This PR enables this functionality across "compatible" schedulers by essentially doing the following:
By "allows" it's meant that no warning is thrown.
The necessary logic for this is implemented in
configuration_utils.py
. Please have a look into thetests/test_config.py
file to understand what the new logic can do.