Any chance class members like self._interrupt could be defined in __init__ across pipelines?

[spezialspezial]

Describe the bug

I think there is no benefit to late initializing here and it puts a burden on the library user that could be easily avoided. Also leads to some confusion as it is uncommon, code inspection flags this. Let me know if I'm missing something.

Reproduction

class WanImageToVideoPipeline:
	def __init__(self):
		pass
	
	def __call__(self, *args, **kwargs):
		self._interrupt = False
		return 23

	@property
	def interrupt(self):
		return self._interrupt
	
pipe = WanImageToVideoPipeline()

def on_async_user_abort_call_me_any_time():
	# check if already interrupted but mid step
	print(pipe.interrupt)


on_async_user_abort_call_me_any_time()

Logs

AttributeError: 'WanImageToVideoPipeline' object has no attribute '_interrupt'. Did you mean: 'interrupt'?

System Info

Diffusers 0.33.0.dev0, Linux, Python 3.10

Who can help?

@yiyixuxu @DN6

[DN6]

Hey @spezialspezial that's a good point. Indeed, it makes more sense that way. Would you be able to open a PR for it? Feel free to tag me to review.

[github-actions]

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.

[ishan-modi]

I see that the same variable initialization is applicable across most of our pipelines and community examples, do we want to change it across all ?

[DN6]

@ishan-modi we can change it for all pipelines in src to start along with any other attributes that are better suited to be created in init.

[ishan-modi]

@DN6, following are all the variables that are exposed as property of instance but are initialized when first call() is made, I will move them all into the init function with below mentioned default. Most of them will be overrided from call apart from a few like interrupt, current_timestep.

_guidance_scale --> defaults to 1.0
_clip_skip --> defaults to None
_interrupt --> defaults to False
_cross_attention_kwargs --> defaults to None
_num_timesteps --> defaults to 0
_attention_kwargs --> defaults to None
_guidance_rescale --> defaults to 0.0
_denoising_start --> defaults to None
_denoising_end  --> defaults to None
_joint_attention_kwargs --> defaults to None
_pag_scale --> defaults to 0.0
_adaptive_pag_scale --> defaults to 0.0
_current_timestep --> defaults to None

Let me know your thoughts !

[ishan-modi]

@DN6, gentle ping

[DN6]

@ishan-modi Yeah those defaults should work.

cc: @yiyixuxu for awareness. Basic idea here is that the current way of setting pipeline attributes during call invocation isn't ideal. e.g This snippet would throw an AttributeError.

from diffusers import DiffusionPipeline

pipe = DiffusionPipeline.from_pretrained("..")
pipe.guidance_scale

Creating properties after the invocation call and then referencing them isn't great. Proposal is to move the attributes @ishan-modi has listed into the Pipeline init and set reasonable defaults that would be overwritten by the ones passed in through call. The Pipeline object is more consistent this way.

[yiyixuxu]

@DN6 sounds good!

[github-actions]

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.