Official callback tests

[asomoza]

What does this PR do?

Tests for the official callbacks introduced in #7761

• official callback test
• multiple official callbacks test
• SD tests
• SD Inpaint tests
• SDXL tests
• IP Adapter tests
• SDXL Controlnet tests
• 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?

[sayakpaul]

Maybe we just test the officially supported callback here?

[asomoza]

can I know here if the pipeline being tested is SD 1.5 or SDXL? Maybe I can check the pipeline class, but still, IMO is a bit inefficient since the official callbacks right now works for a limited number of pipelines.

[sayakpaul]

Oh okay. So, IIUC, you're proposing not to add tests for the official callbacks then?

[asomoza]

Not quite, it's more like asking for guidance on how to do it following the rest of the code base, so maybe I'll split the questions:

• Are we're going to make tests for all the official callbacks (I think there's going to be at least 10), some for IP Adapters, SD, SDXL and probably Pixart and SD3. So it's not that I was proposing not to add tests, just adding more context to make the decision.
• For example, if I want to test the SDXL one, I didn't see any other test that checked the pipeline class. So what would be the best option for diffusers here? create only one test in StableDiffusionXLPipelineFastTests?, on all of them, create a mixin or just write it in the common one? This will be relevant also for the SD 1.5 but not for IP Adapters since they already have a mixin.

[sayakpaul]

Thanks for providing the additional context. I think the following could work but please let me know your thoughts too:

Create something like

diffusers/tests/pipelines/test_pipelines_common.py

Line 221 in fdb05f5

class IPAdapterTesterMixin:

as you proposed and only add to the pipelines that we know are supported with the official callbacks. I think it's perfectly fine to check against the pipeline class name. Or, we could add an instance variable to the pipeline tester class indicating if the official callback test suite should be run for them or not.

[asomoza]

That's what I would have done so I like it, I'll do the other tests then.

[yiyixuxu]

I agree here; we can make a mixin and add to relevant pipeline tests. I think we should at least test SD and SDXL-based pipelines, it will be nice to have more comprehensive coverage to include Kandinsky, pixart, cascade

we also need to figure out a way to find out which callback works with which pipelines

Here in my from_pipe tester mixin, I use this simple logic to figure out which ones are SD-based and which ones are SDXL based, https://github.com/huggingface/diffusers/blob/e0e8c58f64c024e89d125ddb563d0de4cf252dc1/tests/pipelines/test_pipelines_common.py#L626C9-L626C32

[sayakpaul]

Thanks! Maybe we just test the officially supported callbacks?

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

diffusers/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_inpaint.py

Line 1380 in 1221b28

latent_model_input = torch.cat([latent_model_input, mask, masked_image_latents], dim=1)

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 #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]

can we just do "np" here? some of the pipelines does not support "latent" but they should all support "np"

Suggested change

	inputs["output_type"] = "latent"
	inputs["output_type"] = "np"

[yiyixuxu]

left one comment - looks good to me overall!

[yiyixuxu]

@DN6 can you do a final review?