Common API

[LysandreJik]

How much of a common API do you expect there to be across different objects? For example, the unet.py file contains a Trainer, while other models seem to only rely on their forward method, but they have different APIs and signatures.

Same with pipelines, for example with the BDDM pipeline having the following arguments for it's __call__ method: mel_spectrogram, generator, torch_device=None, while the DDIM pipeline has batch_size=1, generator=None, torch_device=None, eta=0.0, num_inference_steps=50.

Do we expect all of these models, pipelines, schedulers to have a common API at the end? Is that even possible with diffusers?

It seems like most arguments are similar, but with a few specificities for models, pipelines and schedulers. That's where having a configuration system would arguably work quite well as it would show very visibly what each of them has in terms of arguments for customization.

This reminds me a bit of the do_lower_case problem we have in transformers: some tokenizers have it, some don't, but users don't necessarily understand that and try to use it for all tokenizers.

[patrickvonplaten]

My take here is that:

• models: All models should follow the same API meaning that all classes defined in the folder should all only provide one and only one forward(...) method, be framework specific, and have more or less the same function signature in forward(...) (conditional models will require one more function argument). This is still a big TODO - we need to clean up this folder.

• schedulers: All models should follow a very similar API and all provide one or multiple step(...) methods that all have a very similar signature. All schedulers should be aligned as much as possible in terms of API and should be framework independant. I don't think we can force them though to have the exact same API (e.g. PNDM requires two loops), but each step method in the loop follows more or less the same signature. This is already done to a big extent - e.g. compare the different files in the above directory to each other.

• pipelines: It'll be harder to force a common API for pipelines given that they will be very different depending on which modality they are used etc... Here, I'd like to force every pipeline to implement one and only one forward(...) method and have pipelines be framework specific, but I don't think we can/should force the function signature to be the same really. The idea is that new pipelines will be closely follow published papers and easy-to-add for a new contributor. Note however that pipelines will be only used for inference.

[patrickvonplaten]

Think this is a very important discussion to have. What do you think @patil-suraj @anton-l @thomwolf ?

[anton-l]

• models: the UNet variations can definitely be consolidated into one. However, I'm in favor of keeping modality-specific subclasses, similar to ...ForTaskName in tranformers. See, for example, GLIDETextToImageUNetModel and GLIDESuperResUNetModel here, where we have different forwards depending on how the model is conditioned.

• schedulers: basically we just need a "forward diffusion process" step() for training and a "backward diffusion process" step() for inference. Hopefully that holds for all models.

• pipelines: We might need to separate __call__ into sub-steps for pipelines with multiple steps, e.g. text2image + upscale1 + upscale2. And the arguments, of course, are very different between implementations. I would even suggest a PipelineConfig class for extremely parameterized pipelines like Majesty Diffusion

[patil-suraj]

• models: Agree with @anton-l here, having modality/task (ex: text/embedding conditioned, class conditioned) specific sub-classes would be nice. This will make the code much more readable.

• schedulers: Pretty much the same comment as Patrick and Anton

• pipelines: Agree with @anton-l here again. Some pipelines do multiple things and users may want to do one or multiple or all of the steps. Having some way of specifying sub-steps would be useful.

[kashif]

One thing that is sometimes missing with the models's forward is the notion of a context or conditioning... typically for the image gen. there is no conditioning, however for other modalities conditioning might be needed so if we can also add that in the forward and by default make it none, it will be helpful!

[patil-suraj]

Thanks @kashif . I think this is exactly what @anton-l proposed with task/modality specific sub-classes.

[patrickvonplaten]

@anton-l RE:

the UNet variations can definitely be consolidated into one. However, I'm in favor of keeping modality-specific subclasses, similar to ...ForTaskName in tranformers. See, for example, GLIDETextToImageUNetModel and GLIDESuperResUNetModel here, where we have different forwards depending on how the model is conditioned.

Haven't looked too much into the models yet, but do we really need them to be modality specific? E.g. couldn't a vision unet also work on a spectrogram image? Think I'd be at the moment more in favor of naming the modules in src/diffusers/models after their input / output format and model type - e.g. unet_2d.py, unet_3d.py. In the case of glide, could that maybe mean that we have a unet_conditional_2d.py and a unnet_upsample.py module? I'm a bit worried that if we add For<TaskName> or For<Modality> too early and then find out later that those models work for other modalities it'll be a bit difficult to refactor them later on. Note here the target group is also more experienced users so they might not need the "guidance" of a modality specific name.

However, for the pipelines I'd be very much in favor of quickly making the distinction between modalities as the pipelines should 1-to-1 reflect architectures proposed in papers. Here the target group is less experienced users which just want to run the model as presented in a paper.

What do you think @patil-suraj @anton-l ?

[patrickvonplaten]

Not super excited about early-on introducing a pipeline config, but happy to brainstorm about it (don't know much about majestic yet :-))

[patrickvonplaten]

Also closing as the API is now finalized and explain on the main README: https://github.com/huggingface/diffusers