Skip to content

[WIP] Diffusers upgrade 0.31.0#1499

Closed
pi314ever wants to merge 2 commits into
huggingface:mainfrom
pi314ever:diffusers-upgrade-0.31.0
Closed

[WIP] Diffusers upgrade 0.31.0#1499
pi314ever wants to merge 2 commits into
huggingface:mainfrom
pi314ever:diffusers-upgrade-0.31.0

Conversation

@pi314ever
Copy link
Copy Markdown
Contributor

@pi314ever pi314ever commented Nov 19, 2024

What does this PR do?

Updates diffusers version to 0.31.0. Changed deprecated sections in EulerDescreteScheduler argument passthrough and get_processor for attention processors in ControlNetSDVModel.

Validated with make fast_tests_diffusers and make slow_tests_diffusers RUN_SLOW=1 (WIP).

Before submitting

  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Signed-off-by: Daniel Huang <daniel1.huang@intel.com>
Signed-off-by: Daniel Huang <daniel1.huang@intel.com>
@pi314ever pi314ever requested a review from regisss as a code owner November 19, 2024 19:49
@pi314ever pi314ever changed the title Diffusers upgrade 0.31.0 [WIP] Diffusers upgrade 0.31.0 Nov 19, 2024
@pi314ever pi314ever marked this pull request as draft November 19, 2024 20:07
@pi314ever
Copy link
Copy Markdown
Contributor Author

I changed to WIP/draft because of failing tests for make slow_tests_diffusers RUN_SLOW=1. Specifically, these are the failing tests:

FAILED tests/test_diffusers.py::GaudiStableDiffusionPipelineTester::test_no_generation_regression - AssertionError: 31.6576 not greater than or equal to 34.9353
FAILED tests/test_diffusers.py::GaudiStableDiffusionPipelineTester::test_no_generation_regression_ldm3d - AssertionError: 31.7588 not greater than or equal to 34.0195

Both of them are failing inconsistently, and they refer to the RGB clip score not meeting the target score. I have also tested this on the main branch and received similar failing results. Should this be a concern for this PR, or should it be address in another issue (most likely by locating sources of randomness and relaxing target score modifier)? Any recommendations/help on this will be greatly appreciated!

@imangohari1
Copy link
Copy Markdown
Contributor

@pi314ever
Hi Daniel,
Thanks for working on this but there is already an open PR to upgrade the diffusers. #1482
We likely don't need this PR separately.

@pi314ever
Copy link
Copy Markdown
Contributor Author

@imangohari1 I notice that there is a slight difference in how that PR handles the changes to ControlNetSDV. Is there a preference for this?

@imangohari1
Copy link
Copy Markdown
Contributor

@imangohari1 I notice that there is a slight difference in how that PR handles the changes to ControlNetSDV. Is there a preference for this?

not necessary a preference but I think @dsocek has removed the unnecessary codes in #1482 .

DanielS did you notice that get_processor is deprecated in your PR? Maybe we need to double check?

@dsocek
Copy link
Copy Markdown
Contributor

dsocek commented Nov 20, 2024

@imangohari1 I notice that there is a slight difference in how that PR handles the changes to ControlNetSDV. Is there a preference for this?

not necessary a preference but I think @dsocek has removed the unnecessary codes in #1482 .

DanielS did you notice that get_processor is deprecated in your PR? Maybe we need to double check?

@imangohari1 @pi314ever ControlNetSDV pipeline does not actually use these attn property functions. This is an experimental pipeline from Ciara Rowles which has not been included in official diffusers so its not maintained (i.e., we cant just inherit it from new versions of diffusers). So I decided to drop these property fns as they are not needed for inference SVD+ControlNet (I checked this before I removed these fns).

@pi314ever
Copy link
Copy Markdown
Contributor Author

@dsocek Thanks for the insight! I think this PR is not relevant anymore.

@pi314ever pi314ever closed this Nov 20, 2024
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.

3 participants