Skip to content

Conversation

tolgacangoz
Copy link
Contributor

@tolgacangoz tolgacangoz commented Mar 12, 2024

Fixes #6126.

This pull request includes three commits:

  • Fix tiling

  • Add docstring to decode method in ConsistencyDecoderVAE

I tested with this:

import torch
from diffusers import ConsistencyDecoderVAE

vae = ConsistencyDecoderVAE.from_pretrained("openai/consistency-decoder").to("cuda")
vae.enable_tiling()
vae.encode(torch.randn((1, 3, 256, 256), device="cuda"))

Also, this PR passes ./tests/models/autoencoders/test_models_vae.py tests in my local setup.

@yiyixuxu @sayakpaul

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

This is looking solid.

I left a major comment on how we should expose the tiling arguments (init vs. via enable_vae_tiling()).

Can we also add a test for this?

@sayakpaul sayakpaul requested a review from yiyixuxu March 12, 2024 14:25
@tolgacangoz tolgacangoz changed the title Fix tiling in ConsistencyDecoderVAE Fix Tiling in ConsistencyDecoderVAE Mar 12, 2024
Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

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

hey! thanks for the PR.
it's looking great!

I left a feedback and additionally, could you add a test_vae_tiling test to ./tests/models/autoencoders/test_models_vae.py( similar to

def test_vae_tiling(self):
)

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Looking good! Let's add a test as well?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@tolgacangoz tolgacangoz marked this pull request as draft March 18, 2024 06:34
@yiyixuxu
Copy link
Collaborator

@StandardAI let us know when you are ready for a final review

@tolgacangoz
Copy link
Contributor Author

It seems to pass the test in Colab.

@tolgacangoz tolgacangoz marked this pull request as ready for review March 24, 2024 16:05
Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

One comment and once addressed, we can ship!

@sayakpaul
Copy link
Member

Please ping me once the CI run is done.

@tolgacangoz
Copy link
Contributor Author

Please ping me once the CI run is done.

@sayakpaul

@sayakpaul sayakpaul merged commit 443aa14 into huggingface:main Mar 26, 2024
@sayakpaul
Copy link
Member

Thank you for this impressive contribution!

@tolgacangoz
Copy link
Contributor Author

Thanks so much for reviewing and merging!

@tolgacangoz tolgacangoz deleted the fix-enable_tiling-for-ConsistencyDecoderVAE branch March 26, 2024 12:43
AbhinavGopal pushed a commit to AbhinavGopal/diffusers that referenced this pull request Mar 27, 2024
* Fix typos

* Add docstring to `decode` method in `ConsistencyDecoderVAE`

* Fix tiling

* Enable tiled VAE decoding with customizable tile sample size and overlap factor

* Revert "Enable tiled VAE decoding with customizable tile sample size and overlap factor"

This reverts commit 1810496.

* Add VAE tiling test for `ConsistencyDecoderVAE`

---------

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
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.

AttributeError: 'ConsistencyDecoderVAE' object has no attribute 'tile_sample_min_size'
4 participants