Skip to content

Conversation

@sgrigory
Copy link
Contributor

Closes #553

DDIM and DDPM pipelines are equivalent only if DDIM scheduler is called with use_clipped_model_output=True. Currently it's not possible to pass this argument from the pipeline, and so test_ddpm_ddim_equality_batched was failing. The unbatched version test_ddpm_ddim_equality was also failing if one changed the random seed (e.g. to 4), as described in the issue.

This PR:

  • allows passing use_clipped_model_output from DDIM pipeline to the scheduler
  • calls DDIM with use_clipped_model_output=True in test_ddpm_ddim_equality_batched and test_ddpm_ddim_equality

As a result, both tests are passing (each runs for 2 values of the seed)
Captura de Pantalla 2022-10-30 a las 14 34 10

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 30, 2022

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

@sgrigory sgrigory changed the title [WIP] Fix tests for equivalence of DDIM and DDPM pipelines Fix tests for equivalence of DDIM and DDPM pipelines Oct 30, 2022
@sgrigory sgrigory marked this pull request as ready for review October 30, 2022 15:08
@anton-l anton-l self-requested a review October 30, 2022 18:01
Copy link
Member

@anton-l anton-l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sgrigory! I've now realized that in my attempt to reproduce your solution I passed use_clipped_model_output incorrectly through the pipeline and it didn't affect the scheduler 😅 It works now!

Gentle ping @patrickvonplaten, as it's a scheduler change

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! Thanks a mille @sgrigory

@patrickvonplaten patrickvonplaten merged commit 5cd29d6 into huggingface:main Nov 2, 2022
PhaneeshB pushed a commit to nod-ai/diffusers that referenced this pull request Mar 1, 2023
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
* Fix equality test for ddim and ddpm

* add docs for use_clipped_model_output in DDIM

* fix inline comment

* reorder imports in test_pipelines.py

* Ignore use_clipped_model_output if scheduler doesn't take it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_ddpm_ddim_equality fails if manual seed changed to 4

4 participants