Unify offset configuration in DDIM and PNDM schedulers #479

jonatanklosko · 2022-09-12T13:58:34Z

Closes #465.

jonatanklosko · 2022-09-12T13:59:22Z

src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion.py

        # set timesteps
-        accepts_offset = "offset" in set(inspect.signature(self.scheduler.set_timesteps).parameters.keys())
-        extra_set_kwargs = {}
-        if accepts_offset:
-            extra_set_kwargs["offset"] = 1
-
-        self.scheduler.set_timesteps(num_inference_steps, **extra_set_kwargs)
+        self.scheduler.set_timesteps(num_inference_steps)


Should we try to override the configuration here, or are you going to update the configuration in the relevant hf repositories?

HuggingFaceDocBuilderDev · 2022-09-12T14:01:47Z

The documentation is not available anymore as the PR was closed or merged.

Add missing variables

src/diffusers/schedulers/scheduling_pndm.py

jonatanklosko · 2022-09-12T14:43:05Z

src/diffusers/schedulers/scheduling_pndm.py

-        alpha_prod_t = self.alphas_cumprod[timestep + 1 - self._offset]
-        alpha_prod_t_prev = self.alphas_cumprod[timestep_prev + 1 - self._offset]


@patrickvonplaten this change breaks this test:

diffusers/tests/test_scheduler.py

Lines 633 to 655 in f4781a0

def test_full_loop_no_noise(self):

scheduler_class = self.scheduler_classes[0]

scheduler_config = self.get_scheduler_config()

scheduler = scheduler_class(**scheduler_config)

num_inference_steps = 10

model = self.dummy_model()

sample = self.dummy_sample_deter

scheduler.set_timesteps(num_inference_steps)

for i, t in enumerate(scheduler.prk_timesteps):

residual = model(sample, t)

sample = scheduler.step_prk(residual, i, sample).prev_sample

for i, t in enumerate(scheduler.plms_timesteps):

residual = model(sample, t)

sample = scheduler.step_plms(residual, i, sample).prev_sample

result_sum = torch.sum(torch.abs(sample))

result_mean = torch.mean(torch.abs(sample))

assert abs(result_sum.item() - 428.8788) < 1e-2

assert abs(result_mean.item() - 0.5584) < 1e-3

However, I think the test must be wrong, because there we use the default offset=0, which means that we are using self.alphas_cumprod[1] for the timestep 0, which doesn't make sense. I assume the value in the test is computed using the current implementation, rather than a reference value from elsewhere?

Hmm, I think the test breaks because set_alpha_to_one defaults to True no?

Actually no, adding + 1 - self.config.steps_offset back fixes the tests, but to me the failure is expected.

I kept the change and adjusted the test value.

src/diffusers/schedulers/scheduling_ddim.py

Co-authored-by: Patrick von Platen <[email protected]>

patrickvonplaten

Super cool PR! Very nice that you've deprecated the offset function argument - that's very nice!

Think we're very close to merging this one :-) Think there are just two things to do:

Make sure the change is fully backwards compatible
Add a couple of tests to DDIM and PNDM that show how set_alpha_to_one and offset should be passed to __init__ and changes the outputs respectively (happy to help here if you want :-) )

jonatanklosko · 2022-09-13T15:33:16Z

tests/test_scheduler.py


            scheduler.step_plms(self.dummy_sample, 1, self.dummy_sample).prev_sample

    def test_full_loop_no_noise(self):


@patrickvonplaten in 394243c you change PNDM step to accept the timestep value rather than iteration number, but I think the tests were kept as is. I changed the full loop to use actual timesteps.

jonatanklosko · 2022-09-13T15:35:04Z

tests/test_scheduler.py

+
    def test_betas(self):
-        for beta_start, beta_end in zip([0.0001, 0.001, 0.01], [0.002, 0.02, 0.2]):
+        for beta_start, beta_end in zip([0.0001, 0.001], [0.002, 0.02]):


I removed this betas combination as it leads to infinite results now with the update to step_prk.

jonatanklosko · 2022-09-13T15:36:50Z

@patrickvonplaten I added some tests, let me know what you think. I hardcoded the values that are currently returned, or do you usually obtain them in a different way?

Make sure the change is fully backwards compatible

Anything I should test in particular?

patrickvonplaten · 2022-09-16T17:24:49Z

src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_img2img.py

        init_latents = torch.cat([init_latents] * batch_size)

        # get the original timestep using init_timestep
+        offset = self.scheduler.config.get("steps_offset", 0)


patrickvonplaten · 2022-09-16T19:06:03Z

src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion.py

        super().__init__()
        scheduler = scheduler.set_format("pt")
+
+        if hasattr(scheduler.config, "steps_offset") and scheduler.config.steps_offset != 1:


@jonatanklosko - Currently the slow tests are broken because we haven't (and can't yet) update the configs online. So for now I think we need to do some deprecation warning here - ok for you?

Perfect!

Our of curiosity, is there an issue with updating the configs now? I assume an additional property in the config doesn't hurt even if ignored by previous versions?

Most users are on the 0.3.0 version therefore will not make use of this code but will nevertheless use the newest config file. Given the high usage of stable diffusion: https://huggingface.co/CompVis/stable-diffusion-v1-4 (>500k downloads per month), I think it would be better to not burden users with an extra config parameter that should throw a warning in 0.3.0 .

So, it would indeed not break anything, but an unused config parameter should throw a warning (haven't checked though whether our code actually works correctly here), which will probably scare users.

Does that make sense?

Ah totally, I didn't realise the extra option would cause a warning :)

patrickvonplaten

Amazing PR!

Sorry for fiddling into your code here @jonatanklosko - only added some code to make all the stability pipelines backwards compatible.
Once this code is in tho 0.4.0 release, we can update all the configs online on the Hub and nudge people to upgrade their diffusers version.

@anton-l @patil-suraj @pcuenca - please have a look here as well. We need to make sure that we remember to update all the configs on the Hub once this PR is in the next release. Also once we update the configs old diffusers versions should throw a warning because of an unused parameter.

patrickvonplaten · 2022-09-16T19:10:12Z

@jonatanklosko - let me if the PR is good for you as it's now!

jonatanklosko · 2022-09-16T20:32:31Z

@patrickvonplaten fantastic, thanks for fixing the pipelines! Looks good to me :)

…ge (#439) * accept tensors * fix mask handling * make device placement cleaner * update doc for mask image

* remove match_shape * ported fixes from #479 to flax * remove unused argument * typo * remove warnings

* [WEB] CSS changes to the web-ui (huggingface#465) This commit updates UI with styling. Signed-Off-by: Gaurav Shukla <[email protected]> Signed-off-by: Gaurav Shukla <[email protected]> * [WEB] Update the title (huggingface#466) * [WEB] Add support for long prompts (huggingface#467) * [WEB] fix background color Signed-Off-by: Gaurav Shukla * [WEB] Remove long prompts support It removes support to long prompts due to higher lag in loading long prompts. Signed-Off-by: Gaurav Shukla <gaurav@nod-labs> * [WEB] Update nod logo and enable debug feature. Signed-Off-by: Gaurav Shukla <[email protected]> Signed-off-by: Gaurav Shukla <[email protected]> Signed-off-by: Gaurav Shukla Signed-off-by: Gaurav Shukla <gaurav@nod-labs>

* Unify offset configuration in DDIM and PNDM schedulers * Format Add missing variables * Fix pipeline test * Update src/diffusers/schedulers/scheduling_ddim.py Co-authored-by: Patrick von Platen <[email protected]> * Default set_alpha_to_one to false * Format * Add tests * Format * add deprecation warning Co-authored-by: Patrick von Platen <[email protected]>

* remove match_shape * ported fixes from huggingface#479 to flax * remove unused argument * typo * remove warnings

Unify offset configuration in DDIM and PNDM schedulers

2e1278f

jonatanklosko commented Sep 12, 2022

View reviewed changes

Format

c727b46

Add missing variables

jonatanklosko force-pushed the jk-scheduler-offset branch from 27a9cd9 to c727b46 Compare September 12, 2022 14:14

jonatanklosko commented Sep 12, 2022

View reviewed changes

src/diffusers/schedulers/scheduling_pndm.py Outdated Show resolved Hide resolved

Fix pipeline test

00eafd1

jonatanklosko commented Sep 12, 2022

View reviewed changes

patrickvonplaten reviewed Sep 13, 2022

View reviewed changes

src/diffusers/schedulers/scheduling_ddim.py Outdated Show resolved Hide resolved

jonatanklosko and others added 2 commits September 13, 2022 15:11

Update src/diffusers/schedulers/scheduling_ddim.py

63c6ed0

Co-authored-by: Patrick von Platen <[email protected]>

Default set_alpha_to_one to false

3f11611

patrickvonplaten reviewed Sep 13, 2022

View reviewed changes

jonatanklosko added 4 commits September 13, 2022 15:15

Format

ea4c848

Add tests

916e535

Merge branch 'main' into jk-scheduler-offset

89b3eec

Format

5afa780

jonatanklosko commented Sep 13, 2022

View reviewed changes

Merge branch 'main' into jk-scheduler-offset

19568b5

patrickvonplaten reviewed Sep 16, 2022

View reviewed changes

patrickvonplaten added 2 commits September 16, 2022 18:06

merge master into PR

aa25955

add deprecation warning

babee70

patrickvonplaten reviewed Sep 16, 2022

View reviewed changes

patrickvonplaten approved these changes Sep 16, 2022

View reviewed changes

patrickvonplaten merged commit d7dcba4 into huggingface:main Sep 17, 2022

patrickvonplaten referenced this pull request Sep 17, 2022

[StableDiffusionInpaintPipeline] accept tensors for init and mask ima…

06924c6

…ge (#439) * accept tensors * fix mask handling * make device placement cleaner * update doc for mask image

jonatanklosko deleted the jk-scheduler-offset branch September 17, 2022 18:09

kashif added a commit to kashif/diffusers that referenced this pull request Sep 19, 2022

ported fixes from huggingface#479 to flax

670da29

kashif mentioned this pull request Sep 19, 2022

[Flax] fix Flax scheduler #564

Merged

kashif added a commit that referenced this pull request Sep 19, 2022

[Flax] fix Flax scheduler (#564)

b1182bc

* remove match_shape * ported fixes from #479 to flax * remove unused argument * typo * remove warnings

yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023

[Flax] fix Flax scheduler (huggingface#564)

707c959

* remove match_shape * ported fixes from huggingface#479 to flax * remove unused argument * typo * remove warnings

pcuenca mentioned this pull request Mar 5, 2024

Change step_offset scheduler docstrings #7128

Merged

		alpha_prod_t = self.alphas_cumprod[timestep + 1 - self._offset]
		alpha_prod_t_prev = self.alphas_cumprod[timestep_prev + 1 - self._offset]

	def test_full_loop_no_noise(self):
	scheduler_class = self.scheduler_classes[0]
	scheduler_config = self.get_scheduler_config()
	scheduler = scheduler_class(**scheduler_config)

	num_inference_steps = 10
	model = self.dummy_model()
	sample = self.dummy_sample_deter
	scheduler.set_timesteps(num_inference_steps)

	for i, t in enumerate(scheduler.prk_timesteps):
	residual = model(sample, t)
	sample = scheduler.step_prk(residual, i, sample).prev_sample

	for i, t in enumerate(scheduler.plms_timesteps):
	residual = model(sample, t)
	sample = scheduler.step_plms(residual, i, sample).prev_sample

	result_sum = torch.sum(torch.abs(sample))
	result_mean = torch.mean(torch.abs(sample))

	assert abs(result_sum.item() - 428.8788) < 1e-2
	assert abs(result_mean.item() - 0.5584) < 1e-3


		scheduler.step_plms(self.dummy_sample, 1, self.dummy_sample).prev_sample

		def test_full_loop_no_noise(self):

Unify offset configuration in DDIM and PNDM schedulers #479

Unify offset configuration in DDIM and PNDM schedulers #479

Uh oh!

Conversation

jonatanklosko commented Sep 12, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HuggingFaceDocBuilderDev commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatanklosko Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patrickvonplaten left a comment

Choose a reason for hiding this comment

Uh oh!

jonatanklosko Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatanklosko commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten commented Sep 16, 2022

Uh oh!

jonatanklosko commented Sep 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HuggingFaceDocBuilderDev commented Sep 12, 2022 •

edited

Loading

jonatanklosko Sep 13, 2022 •

edited

Loading

jonatanklosko Sep 13, 2022 •

edited

Loading

jonatanklosko commented Sep 13, 2022 •

edited

Loading

patrickvonplaten left a comment •

edited

Loading