Official callback tests

[asomoza]

What does this PR do?

Tests for the official callbacks introduced in #7761

• [X] official callback test
• [ ] multiple official callbacks test
• [X] SD tests
• [ ] SD Inpaint tests
• [X] SDXL tests
• [ ] IP Adapter tests
• [X] SDXL Controlnet tests
• [X] SD Controlnet tests

Who can review?

@sayakpaul @yiyixuxu

[asomoza]

@yiyixuxu @sayakpaul

I used the normal callback test as a guide, are this tests enough or should I add more?

Also for the 3 official callbacks we have right now, should I add test cases for the specific pipelines? What I mean is if we're going to test all the official callbacks implementations?

[asomoza]

I'll do the kandinsky special test at the end which is the one failing.

[asomoza]

While doing the tests I'm getting an error if I do the test with StableDiffusionInpaintPipeline

The error happens in this line:

https://github.com/huggingface/diffusers/blob/1221b28eac1801dd759e8d1df9fc9a2a998b41ed/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_inpaint.py#L1380

Since the SDCFGCutoffCallback makes the guidance_scale 0.0, I will also need to slice the mask and masked_image_latents but this only happens for this pipeline and the pix2pix one.

I think we have this options:

• Create an official callback just for these pipelines
• Skip the test for these pipelines

@sayakpaul @yiyixuxu WDYT?

Is there an alternative that I'm missing?

[sayakpaul]

I think both the pipelines are important. So, I won't prefer skipping them. Having separate callbacks (Inpainting, Pix2Pix, etc.) seems like the best and cleanest way here. @yiyixuxu WDYT?

[yiyixuxu]

@sayakpaul @asomoza agree I don't think we should just skip but we can open that task for community! so alternative is we can skip them for now and open an issue to ask the community to add these separate callbacks (like @sayakpaul did here https://github.com/huggingface/diffusers/issues/7677) , whichever way is easier for you:)

[yiyixuxu]

ohhhh another way is just handle this in the callbacks. I think you can actually combine the SD and SDXL callbacks with if "add_text_embeds" in callback_kwargs: ...

[yiyixuxu]

yeah I think try to make the callback work for all pipeline is better, this way it's easier for tests too

[asomoza]

yeah, I'll try to do them all in this PR.

[asomoza]

ohhhh another way is just handle this in the callbacks. I think you can actually combine the SD and SDXL callbacks with if "add_text_embeds" in callback_kwargs: ...

Now that I analyze this, for the add_text_embeds to be in callback_kwargs I need to add them to the tensor_inputs, but this errors if I add them to the list and aren't in the _callback_tensor_inputs

For this to work I would need to bypass that check and allow the use of any name in tensor_inputs for the official callbacks.

[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.

[yiyixuxu]

@DN6 can you do a final review?

[DN6]

Sorry I missed this. Left a comment on a spelling mistake and I believe @yiyixuxu's comment still needs to be addressed. https://github.com/huggingface/diffusers/pull/7930/files#r1618014761

Good to merge once conflicts are resolved and tests pass.

[asomoza]

I changed the output_type to "np" instead of "latents" but this means I have to validate against a numpy array that has been decoded by the vae. This value changes too much between model types and if it's a img2img pipeline, so I'll probably need to do all of the test cases in this PR.

Don't merge yet.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.