Skip to content

Update pipeline_stable_diffusion_inpaint_legacy.py#1583

Closed
Randolph-zeng wants to merge 2 commits intohuggingface:mainfrom
Randolph-zeng:patch-2
Closed

Update pipeline_stable_diffusion_inpaint_legacy.py#1583
Randolph-zeng wants to merge 2 commits intohuggingface:mainfrom
Randolph-zeng:patch-2

Conversation

@Randolph-zeng
Copy link
Copy Markdown
Contributor

This PR is related to the issue #1581. Please refer to it as why for the proposed PR, if more extensive local tests needed please let me know : )

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@Randolph-zeng
Copy link
Copy Markdown
Contributor Author

btw, It seems tha there are some errors regarding compatability and formatting, if someone can confirm that this is indeed an undesired off by one error I am happy to follow the contributing procedure to run local test to fix those CI errors

@averad
Copy link
Copy Markdown

averad commented Dec 9, 2022

Any thoughts about adding the changes to the Onnx Pipeline as well?

pipeline_onnx_stable_diffusion_inpaint_legacy.py - Line 374

# add noise to latents using the timesteps
noise = generator.randn(*init_latents.shape).astype(latents_dtype)
t_minus_one = timesteps - self.scheduler.config.num_train_timesteps // self.scheduler.num_inference_steps
if t_minus_one > 0:
	init_latents = self.scheduler.add_noise(
		torch.from_numpy(init_latents), torch.from_numpy(noise), torch.from_numpy(t_minus_one)
	)
else:
	init_latents = self.scheduler.add_noise(
		torch.from_numpy(init_latents), torch.from_numpy(noise), torch.from_numpy(timesteps)
	)
init_latents = init_latents.numpy()

Not 100% sure I incorporated the changes correctly

@Randolph-zeng
Copy link
Copy Markdown
Contributor Author

oh yeah, thanks for the catch! I will incorporate that and fix the errors by doing some local test

@averad
Copy link
Copy Markdown

averad commented Dec 9, 2022

Code change is similar to PR #1585 - Linking so both are reviewed at the same time.

@averad
Copy link
Copy Markdown

averad commented Dec 11, 2022

@patrickvonplaten
Copy link
Copy Markdown
Contributor

patrickvonplaten commented Dec 15, 2022

I'm not sure this change is correct here @Randolph-zeng - can you explain a bit more why you think the timestep should be changed? Also, the inpaint stable diffusion legacy pipeline really isn't an official pipeline because there is no reference paper. I'm not sure it should follow the RePaint example . @anton-l wdyt?

@Randolph-zeng
Copy link
Copy Markdown
Contributor Author

Hi @patrickvonplaten , I totally agree with you that SD-Legacy is not a standard pipeline following any paper and is different from RePaint. It is just that every diffusion models are trained with fixed noise schedule that associates with the timestamps. If you are using a sampler that samples in 20 steps instead of 1000 steps, then mixing parts of images that are 50 steps away from each other might result in a very different input than the models were trained with, and thus deteriorate the model performance.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 9, 2023

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.

@github-actions github-actions Bot added the stale Issues that haven't received updates label Jan 9, 2023
@github-actions github-actions Bot closed this Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Issues that haven't received updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants