[WIP] SDXL ControlNet pipeline follow-up fixes #4155

gkorepanov · 2023-07-19T12:05:59Z

What does this PR do?

Follow-up for #4038 with fixes which allow switching CFG and "guess_mode" in SD XL controlnet pipeline

Who can review?

@sayakpaul can you please suggest if I need to add some tests here? Also you mentioned that this bug blocks multi controlnet support, should I also try to add it in this PR?

HuggingFaceDocBuilderDev · 2023-07-19T12:14:03Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

examples/controlnet/train_controlnet_sdxl.py

src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl.py

sayakpaul · 2023-07-20T02:41:28Z

Also you mentioned that this bug blocks multi controlnet support, should I also try to add it in this PR?

@gkorepanov sure, let's try to merge that in.

src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl.py

sayakpaul · 2023-07-20T02:44:06Z

src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl.py

+                scaled_latents = self.scheduler.scale_model_input(latents, t)
+                latent_model_input = torch.cat([scaled_latents] * 2) if do_classifier_free_guidance else scaled_latents


Could I have an explanation on why this differs from

diffusers/src/diffusers/pipelines/controlnet/pipeline_controlnet.py

Line 924 in 6b1abba

latent_model_input = torch.cat([latents] * 2) if do_classifier_free_guidance else latents

?

Just wanted to make the code around switching CFG more clear, I don't see the point behind scaling latents once for main unet and then the second time for controlnet. So I made a change which is aligned with all inputs (prompt embeds, text_embeds, time_ids): we first prepare the standard inputs for controlnet, and then expand them (torch.cat([...]*2) for unet if it is required by CFG

Got it. Clean.

I just took a closer look here. Is it possible to do exactly the same as here? https://github.com/huggingface/diffusers/blob/80871ac5971fe7e708befa3b553463c4e61b22ab/src/diffusers/pipelines/controlnet/pipeline_controlnet.py#L938C19-L938C19

I think the logic is very clear there:

all the code create inputs for the controlnet_model is addressed within the if ... else ... statement

it is very clear to me that only when guess_mode and do_classifier_free_guidance it will differ from our regular Unet model input

sayakpaul · 2023-07-20T03:00:03Z

@gkorepanov this is a great start!

Let's add a few test cases to ensure feature robustness :)

Also, we need to ensure the existing tests don't fail. This is from the CI:

=========================== short test summary info ============================
FAILED tests/pipelines/controlnet/test_controlnet_sdxl.py::ControlNetPipelineSDXLFastTests::test_inference_batch_consistent - RuntimeError: mat1 and mat2 shapes cannot be multiplied (4x56 and 80x128)
FAILED tests/pipelines/controlnet/test_controlnet_sdxl.py::ControlNetPipelineSDXLFastTests::test_inference_batch_single_identical - RuntimeError: mat1 and mat2 shapes cannot be multiplied (6x48 and 80x128)
FAILED tests/pipelines/controlnet/test_controlnet_sdxl.py::ControlNetPipelineSDXLFastTests::test_num_images_per_prompt - RuntimeError: mat1 and mat2 shapes cannot be multiplied (4x56 and 80x128)
==== 3 failed, 1061 passed, 625 skipped, 1380 warnings in 752.68s (0:12:32) ====

sayakpaul · 2023-07-31T04:33:18Z

@gkorepanov let us know if there's anything we can do to accelerate this PR :-)

gkorepanov · 2023-08-02T16:37:10Z

@gkorepanov let us know if there's anything we can do to accelerate this PR :-)

Sorry for long delay, my availability has been limited recently. I've managed to address the issues with current tests. However, I'm uncertain about adding new tests since we lack an official ControlNet model for SDXL to run a comprehensive test. So I've merely added tests to verify the pipeline's basic functionality with disabled CFG and enabled guess mode: https://github.com/huggingface/diffusers/pull/4155/files#diff-fa8f17e303ce826636a7e4038a33ea652b1f9279cce289ddaacb4332efd0b4f9R263

Please let me know if I should add new tests in some different way.

Regarding multi-controlnet, I'm unsure if I'll be able to dedicate time to work on it in the near future :(

sayakpaul · 2023-08-03T04:00:40Z

Will get to reviewing this, soon! Thank you for your hard work!

sayakpaul · 2023-08-07T04:05:22Z

@gkorepanov thanks for your efforts! The PR looks good to me.

However, I'm uncertain about adding new tests since we lack an official ControlNet model for SDXL to run a comprehensive test.

I created this dummy ControlNet pipeline: https://huggingface.co/hf-internal-testing/dummy-sdxl-controlnet-pipe. Will this suffice for testing?

sayakpaul · 2023-08-14T03:46:04Z

Hey @gkorepanov. I think the PR looks good in its current form. Let me also check with @yiyixuxu. Yiyi, could you also give this a look?

yiyixuxu

Thanks for the PR!

yiyixuxu · 2023-08-14T21:34:54Z

tests/pipelines/controlnet/test_controlnet_sdxl.py

        assert np.abs(image_slice_1.flatten() - image_slice_2.flatten()).max() < 1e-4
+
+
+class ControlNetPipelineSDXLGuessModeFastTests(ControlNetPipelineSDXLFastTests):


Can we test all four scenarios here?

guess_mode == True & CFG == True

guess_model ==False and CFG == True

guess_model == False & CFG == False

guess_mode == True and CFG == False

I found the logic a little bit complex so let's first add test here to make sure it works as expected here first

Hi, If you have any idea on how we could simplify the logic, it would be welcome!

Regarding tests, do you think it is sufficient to run functional tests (i.e. run pipeline with all combinations of parameters and make sure it does not fail) or we need to run heavy tests with real checkpoint loading and checking the outputs?

Functional tests are fine here.

@sayakpaul
Added tests for all combinations of params: 87b7cfe

I was not sure how to add multiple options given unittest limited functionality in parametrisation, so feel free to suggest better options

I think we can make use of parameterized here no? A couple of our test cases use it.

Let me know.

Let me know.

I have already used it, please have a look at the commit 87b7cfe.

Though parameterized had a bug related to classes inheritance, I have used a workaround, but I don't like it too much

sayakpaul · 2023-08-19T00:52:10Z

@yiyixuxu @DN6 could you give this a look? I think this is important as it fixes support for guess mode in SDXL ControlNet.

yiyixuxu · 2023-08-27T05:46:16Z

@gkorepanov
i made a PR here #4799 - I think it's doing the same thing, but let me know if it's not ;)

If it is possible, we want to keep the logic of how we handle the guess_mode in the current implementation, and I explained my reason here #4155 (comment)

gkorepanov · 2023-08-28T13:30:49Z

If it is possible, we want to keep the logic of how we handle the guess_mode in the current implementation

No problem, up to you. Closing in favour of #4799

patrickvonplaten reviewed Jul 19, 2023

View reviewed changes

examples/controlnet/train_controlnet_sdxl.py Outdated Show resolved Hide resolved

patrickvonplaten reviewed Jul 19, 2023

View reviewed changes

src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl.py Outdated Show resolved Hide resolved

sayakpaul reviewed Jul 20, 2023

View reviewed changes

src/diffusers/pipelines/controlnet/pipeline_controlnet_sd_xl.py Show resolved Hide resolved

sayakpaul reviewed Jul 20, 2023

View reviewed changes

gkorepanov added 8 commits August 2, 2023 15:13

Do not force VAE upcast if custom VAE is used

0fc8ca3

Support switching cfg in controlnet pipeline

2ad7b7e

typo

179a7fe

Fixes from review

6ed6a3f

Fix tests

5be9e23

Add tests for guess mode and no cfg in controlnet SDXL

acf965c

Fix case with unbound local

33db8e5

Disable unrelevant test

46ee54d

style

79cff48

sayakpaul added 2 commits August 14, 2023 08:54

Merge branch 'main' into sdxl-controlnet-fixes

9b25fd2

style.

f59a1db

sayakpaul requested a review from yiyixuxu August 14, 2023 03:46

yiyixuxu reviewed Aug 14, 2023

View reviewed changes

gkorepanov added 2 commits August 18, 2023 19:01

Add tests for all combinations of guess mode and cfg scale

87b7cfe

Merge branch 'main' into sdxl-controlnet-fixes

edfbb60

sayakpaul requested a review from yiyixuxu August 21, 2023 13:49

sayakpaul mentioned this pull request Aug 23, 2023

Fix guess_mode in StableDiffusionXLControlNetPipeline #4725

Closed

yiyixuxu mentioned this pull request Aug 27, 2023

fix bug in StableDiffusionXLControlNetPipeline when use guess_mode #4799

Merged

gkorepanov closed this Aug 28, 2023

		scaled_latents = self.scheduler.scale_model_input(latents, t)
		latent_model_input = torch.cat([scaled_latents] * 2) if do_classifier_free_guidance else scaled_latents

		assert np.abs(image_slice_1.flatten() - image_slice_2.flatten()).max() < 1e-4


		class ControlNetPipelineSDXLGuessModeFastTests(ControlNetPipelineSDXLFastTests):

[WIP] SDXL ControlNet pipeline follow-up fixes #4155

[WIP] SDXL ControlNet pipeline follow-up fixes #4155

Conversation

gkorepanov commented Jul 19, 2023

What does this PR do?

Who can review?

Uh oh!

HuggingFaceDocBuilderDev commented Jul 19, 2023

Uh oh!

Uh oh!

Uh oh!

sayakpaul commented Jul 20, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yiyixuxu Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayakpaul commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sayakpaul commented Jul 31, 2023

Uh oh!

gkorepanov commented Aug 2, 2023

Uh oh!

sayakpaul commented Aug 3, 2023

Uh oh!

sayakpaul commented Aug 7, 2023

Uh oh!

sayakpaul commented Aug 14, 2023

Uh oh!

yiyixuxu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkorepanov Aug 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayakpaul commented Aug 19, 2023

Uh oh!

yiyixuxu commented Aug 27, 2023

Uh oh!

gkorepanov commented Aug 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yiyixuxu Aug 23, 2023 •

edited

Loading

sayakpaul commented Jul 20, 2023 •

edited

Loading

gkorepanov Aug 17, 2023 •

edited

Loading