Reduce huge amounts of code duplication

[zzbuzzard]

Is your feature request related to a problem? Please describe.
There is a ridiculous amount of code duplication going on in this library. Could anyone please tell me why the schedulers function betas_for_alpha_bar is duplicated - identically - 23 times, appearing in about half of the schedulers files? There are many other duplicated functions such as retrieve_timesteps, appearing 6 times. Many many member functions of classes are also duplicated. This seems like extremely bad practice, and makes it very hard to modify these functions.

This is just the tip of the iceberg - there are similar patterns across the whole codebase. The string "# Copied from" occurs in 149 files, and often many times in each file too. Bad sign. Why are there separate standalone files for StableDiffusionPipeline, StableDiffusionImg2ImgPipeline, StableDiffusionInpaintPipeline, StableDiffusionPipelineSafe and so on (and so on and so on) when they have a huge amount in common, and could be expressed as single methods of or even arguments to one pipeline? Even if you want them to be separate classes, surely they should be sharing code via OOP rather than using this huge amount of duplicated code? Surely, functionally, StableDiffusionControlNetImg2ImgPipeline could have been expressed as a very simple combination of the existing ControlNet and Img2Img implementations, rather than being its own 1269 line file (with 17 occurrences of "# Copied from")?

In my opinion this methodology will make this codebase increasingly difficult to maintain and contribute to as the features continue to grow. While I unfortunately don't currently have time to attempt this myself, I really believe this package needs a serious refactor. There are already problems with advanced use; if I were to train a ControlNet on an image variation model, I would be unable to use the result with diffusers as StableDiffusionControlNetImageVariationPipeline does not yet exist (not that it should!).

It's worth running pipelines/stable_diffusion/pipeline_stable_diffusion.py and pipelines/stable_diffusion/pipeline_stable_diffusion_img2img.py through diff to see the extent of the issue. The code and documentation are extremely similar, with huge identical sections. check_inputs is slightly different, Img2Img has a new function get_timesteps, there are slight differences to the arguments to __call__, and slight differences to the body - and that's it. Both files are ~1000 lines long.

Describe the solution you'd like.
Share code between pipelines and schedulers wherever possible. There should surely be some kind of StableDiffusionPipeline base class given the number of related classes. ControlNet should be incorporated in to existing Stable Diffusion pipelines - to my understanding wherever SD is used, ControlNet may also be used (we just need to provide an image condition), so to me it seems very overkill to have separate pipelines for every ControlNet use case. For example, StableDiffusionControlNetImg2ImgPipeline could inherit from StableDiffusionImg2ImgPipeline, or StableDiffusionImg2ImgPipeline could take optional image conditions as input to its call.

Of course these are just suggestions, it's worth thinking very carefully about how to proceed! But I really believe that change is necessary or this library will become more and more bloated as new features are added.

I was originally intending to add a custom beta schedule to this library (the sqrt schedule from Diffusion-LM), but the amount of code duplication that would be necessary for this really put me off. I believe the noise schedule system needs a rewrite - there should really be just one function which maps from a string (and num timesteps etc) to a list of betas, rather than 23 identical functions + additional logic in many different constructors. (perhaps I should separate this into a separate issue?)

[asomoza]

When I started using diffusers a couple of months ago I had the same thought but then I read the philosophy and this blog post.

Since its intentional I just use this library for the models and started designing my own pipelines, recently I even changed everything to a node based system and there's a couple of big projects that do the same.

They even say it in their philosophy:

Pipelines are designed to be easy to use (therefore do not follow Simple over easy 100%), are not feature complete, and should loosely be seen as examples of how to use models and schedulers for inference.

So when you need more advanced features you'll just start using this library less and start coding your own solution, its a great place to learn and start using diffusion models.

[zzbuzzard]

Hey thanks for the reply + links @asomoza! It's interesting to read more about this philosophy - while I don't entirely agree with it personally, it's good to understand the motivation behind it. I have also been using my own pipeline and I agree the pipelines are helpful resources as complete usage examples.

While these links answer a lot of the questions I asked here, I still don't think duplicating utility functions in many different places is very helpful to the community. For the specific case which drew me to raise this issue, I think it's likely that the community will want to design new noise schedules, but unlikely that they will need new versions of the utility function betas_for_alpha_bar. In my opinion, the scheduling system would be more "Tweakable, contributor-friendly" if there was a simple shared function to obtain a noise schedule from a string, so I could tweak this function, rather than having to change it for each of DDPM, DPM++ etc.

FWIW I also think sharing code between e.g. SD pipelines would make it easier to tweak the library - if I want my own custom SD pipeline I'd rather inherit from a base class which provides many relevant utility functions than start by copy pasting a 1000+ line file then changing 10 lines - but apparently this is a matter of preference. I'll close this issue as it's totally at odds with the design philosphy, but might come back to the specific issue of noise schedules later.

Cheers!

[dg845]

Note that # Copied from comments have a special meaning in diffusers: a function or method which is # Copied from should be the same as the function or method it was # Copied from, and this is enforced by the CI. For example, you can expect that diffusers.schedulers.scheduling_ddim.betas_for_alpha_bar

diffusers/src/diffusers/schedulers/scheduling_ddim.py

Lines 50 to 51 in 4836cfa

	# Copied from diffusers.schedulers.scheduling_ddpm.betas_for_alpha_bar
	def betas_for_alpha_bar(

should be exactly the same as the "canonical" version diffusers.schedulers.scheduling_ddpm.betas_for_alpha_bar. If you want to make a change to the betas_for_alpha_bar function and propagate it to all copies, you can alter the canonical copy at diffusers.schedulers.scheduling_ddpm.betas_for_alpha_bar and use

make fix-copies

to propagate the changes. The documentation on # Copied from can be found here.

[sayakpaul]

Hey all! One of the maintainers of the library here :)

We have started doing a bit of refactoring work already to systematically reduce some boilerplate and duplicated code. We are trying to do this while being respectful of the core philosophy of the library (VVI to keep in mind). Some refactoring examples are provided below:

• [Refactor] FreeInit for AnimateDiff based pipelines #6874
• refactor: move model helper function in pipeline to a mixin class #6571
• add a from_pipe method to DiffusionPipeline #7241
• [LoRA test suite] refactor the test suite and cleanse it #7316