huggingface / optimum-habana

Easy and lightning fast training of 🤗 Transformers on Habana Gaudi processor (HPU)
Apache License 2.0
153 stars 202 forks source link

[WIP] Diffusers upgrade 0.31.0 #1499

Closed pi314ever closed 2 days ago

pi314ever commented 3 days ago

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

pi314ever commented 3 days ago

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 commented 3 days ago

@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 commented 3 days ago

@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 commented 3 days ago

@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 commented 2 days ago

@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 commented 2 days ago

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