Skip to content

Conversation

@LagPixelLOL
Copy link
Contributor

@LagPixelLOL LagPixelLOL commented Sep 26, 2024

Fixed local variable noise_pred_text referenced before assignment when using PAG with guidance scale and guidance rescale at the same time.

@yiyixuxu @asomoza

I'm not sure if allowing guidance rescale with PAG + guidance scale is actually a thing, but from my tests it does work with PAG + guidance scale. If it's not allowed then it can be fixed with adding a condition or raising an error when all PAG scale, guidance scale, and guidance rescale are passed at the same time.

Before this fix it currently does raise an error when passing all 3 at the same time, but it's an unbounded local variable error, which is extremely confusing.

…n using PAG with guidance scale and guidance rescale at the same time.
@LagPixelLOL
Copy link
Contributor Author

@sayakpaul

@sayakpaul
Copy link
Member

Thanks for your PR, could you provide a reproducible snippet that raises the error?

@sayakpaul
Copy link
Member

I am also going to cc @sunovivid (PAG first author) to brainstorm if allowing guidance rescale with PAG + guidance scale makes sense.

@LagPixelLOL
Copy link
Contributor Author

import torch
import diffusers

pipeline = diffusers.AutoPipelineForText2Image.from_pretrained("stabilityai/stable-diffusion-xl-base-1.0", torch_dtype=torch.bfloat16, enable_pag=True)
pipeline.to("cuda")
pipeline(
    prompt="test",
    guidance_scale=5,
    guidance_rescale=0.5,
    pag_scale=3,
    num_inference_steps=35,
)

produces the below error

Traceback (most recent call last):
  File "/root/differr.py", line 6, in <module>
    pipeline(
  File "/usr/local/lib/python3.12/site-packages/torch/utils/_contextlib.py", line 116, in decorate_context
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/diffusers/pipelines/pag/pipeline_pag_sd_xl.py", line 1250, in __call__
    noise_pred = rescale_noise_cfg(noise_pred, noise_pred_text, guidance_rescale=self.guidance_rescale)
                                               ^^^^^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'noise_pred_text' where it is not associated with a value

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Oct 7, 2024

same issue reported here #9220

fix looks good to me!

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Oct 7, 2024

can you run make style and make fix-copies?

@yiyixuxu yiyixuxu added the PAG label Oct 7, 2024
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sunovivid
Copy link
Contributor

sunovivid commented Oct 8, 2024

Thank you for the contributions and experiments!

I am also going to cc @sunovivid (PAG first author) to brainstorm if allowing guidance rescale with PAG + guidance scale makes sense.

It looks reasonable to use guidance rescale with PAG + guidance scale. Noise rescale modifies the scale of the 'guided' prediction to the original text-conditioned prediction to cut off extra scale from guidance extrapolation. So rescaling PAG + CFG guided prediction to the original prediction looks reasonable. After the cut-off, the noise rescale function mixes the rescaled noise with the original 'text' prediction. It seems also reasonable because our main component is 'text' prediction, both in CFG and PAG. I think it is a very good fix!

cc @sayakpaul

@LagPixelLOL LagPixelLOL requested a review from yiyixuxu October 8, 2024 08:39
@LagPixelLOL
Copy link
Contributor Author

The style is fixed now.

noise_pred_text, noise_pred_perturb = noise_pred.chunk(2)
noise_pred = noise_pred_text + pag_scale * (noise_pred_text - noise_pred_perturb)
return noise_pred
return noise_pred, noise_pred_text
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like we broke the tests; I looked up the import of PAGMixin on github, not many but some so let's try to not making a breaking change here

we can add a argument return_pred_text; default to False but set True when calling from our pipelines.

@LagPixelLOL LagPixelLOL requested a review from yiyixuxu October 8, 2024 18:42
@LagPixelLOL
Copy link
Contributor Author

Should be fixed now.

@yiyixuxu yiyixuxu merged commit 86bd991 into huggingface:main Oct 8, 2024
15 checks passed
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Oct 8, 2024

@LagPixelLOL merged! thanks for your PR!

leisuzz pushed a commit to leisuzz/diffusers that referenced this pull request Oct 11, 2024
* Fixed local variable noise_pred_text referenced before assignment when using PAG with guidance scale and guidance rescale at the same time.

* Fixed style.

* Made returning text pred noise an argument.
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
* Fixed local variable noise_pred_text referenced before assignment when using PAG with guidance scale and guidance rescale at the same time.

* Fixed style.

* Made returning text pred noise an argument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants