[docs] Fix SDXL docstring by stevhliu · Pull Request #4397 · huggingface/diffusers · GitHub
Skip to content

[docs] Fix SDXL docstring - #4397

Merged
stevhliu merged 1 commit into
huggingface:mainfrom
stevhliu:fix-docstring
Aug 1, 2023
Merged

stevhliu merged 1 commit into
huggingface:mainfrom
stevhliu:fix-docstring

Conversation

@stevhliu

Copy link
Copy Markdown
Member

Fixes #4391 where the guidance_scale should be 5.0 instead of 7.5.

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Jul 31, 2023 •

Copy link
Copy Markdown

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

@rolux

rolux commented Jul 31, 2023

Copy link
Copy Markdown

Is there a rationale why this was changed from 7.5 to 5.0? IMO, the documentation was correct and the actual default was wrong. The default used to be 7.5 in previous versions, and SD literature always refers to "between 7 and 10" as the optimal value.

@stevhliu

stevhliu commented Aug 1, 2023

Copy link
Copy Markdown
Member Author

In the paper, it says they used 5 (see the last paragraph in section 2.2) :)

@stevhliu
stevhliu merged commit 6c5bd2a into huggingface:main Aug 1, 2023
@stevhliu
stevhliu deleted the fix-docstring branch August 1, 2023 23:40
@rolux

rolux commented Aug 2, 2023 •

Copy link
Copy Markdown

Thanks for the info! However, the scale of 5.0 appears only once in the paper, in the specific context of conditioning on image size. There are four instances of example image outputs in the paper, on page 5, 9, 16 and 17, and in all of these cases, the scale used is 8.0. (EDITED TO ADD: In https://github.com/Stability-AI/generative-models/blob/main/sgm/inference/api.py, the default value is 6.0. Having rendered thousands of images at various cfg_scales, my impression is that a value between 6.0 and 8.0 is preferable to 5.0 across a wide variety of subjects.)

I'm still convinced that you are erroneously adopting a wrong default value here, and that your original documentation was correct.

@stevhliu

stevhliu commented Aug 2, 2023

Copy link
Copy Markdown
Member Author

Oh interesting, thanks for providing the additional info! Pinging @patrickvonplaten or @sayakpaul who can help provide more insight into why we chose 5.0

@patrickvonplaten

Copy link
Copy Markdown
Contributor

The original documentation came from a bad copy-paste. Let's ask stability what they think is best here :-)

@patrickvonplaten

Copy link
Copy Markdown
Contributor

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.

Wrong default value (or documentation) in StableDiffusionXLPipeline

4 participants