Skip to content

Conversation

@fabiorigano
Copy link
Contributor

What does this PR do?

Fixes #6180

Who can review?

@yiyixuxu
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

self.config.encoder_hid_dim_type = None

# restore original Unet layers
device = self.unet.device
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking, we just have to set the attention processor back to default here instead of reloading the UNet again.

cc @sayakpaul here, let me know what you think!

Copy link
Member

Choose a reason for hiding this comment

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

Indeed that should be the way to go. We can first store the original attention processors and wap them back in during the unloading process.

Here's a reference that might be relevant:

if self.original_attn_processors is not None:

@yiyixuxu yiyixuxu requested a review from sayakpaul December 15, 2023 18:48
```
"""
# remove CLIP image encoder
if hasattr(self, "image_encoder"):
Copy link
Member

Choose a reason for hiding this comment

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

Might be better:

if hasattr(self, "image_encoder") and getattr(self, "image_encoder", None) is not None:

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.

Thanks for taking the lead on this!

Have left some comments and I agree with @yiyixuxu that https://github.com/huggingface/diffusers/pull/6192/files#r1428359297 should be the way to do it.

@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.

self.config.encoder_hid_dim_type = None

# restore original Unet attention processors layers
self.unet._restore_attn_processors()
Copy link
Member

Choose a reason for hiding this comment

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

This works for me!

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.

Thanks for the changes! Could we add some tests here?

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.

Looks very nice! Let's maybe just remove the original_attn_processors variable for now to not introduce a new logic

Comment on lines 689 to 694
# store original attn_processors if not already stored
if (
getattr(self, "original_attn_processors", None) is None
or getattr(self, "original_attn_processors", None) == {}
):
self.original_attn_processors = self.attn_processors
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# store original attn_processors if not already stored
if (
getattr(self, "original_attn_processors", None) is None
or getattr(self, "original_attn_processors", None) == {}
):
self.original_attn_processors = self.attn_processors

Can we maybe not add this mechanism for now? When unloading the IP adapters let's just set the default SDPA (torch 2.0) attention processors. This introduces too much magic for now IMO

@fabiorigano
Copy link
Contributor Author

@sayakpaul @yiyixuxu in the end I followed @patrickvonplaten's directions on restoring the default attention processors

Added one simple test

if isinstance(attn_processor, (IPAdapterAttnProcessor, IPAdapterAttnProcessor2_0)):
attn_processor.scale = scale

def unload_ip_adapter(self):
Copy link
Member

Choose a reason for hiding this comment

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

Works for me!

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.

Very nice work!

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.

thank you!

@patrickvonplaten
Copy link
Contributor

Great job @fabiorigano

@patrickvonplaten patrickvonplaten merged commit 86714b7 into huggingface:main Jan 2, 2024
@vladmandic
Copy link
Contributor

i think there may be sideeffects of using this - it removes any cross-attention applied to unet

self.unet.set_default_attn_processor()

e.g., if unet was previously using sdpa, after unload it will not.

@fabiorigano
Copy link
Contributor Author

i think there may be sideeffects of using this - it removes any cross-attention applied to unet

self.unet.set_default_attn_processor()

e.g., if unet was previously using sdpa, after unload it will not.

yes, attention processors with torch sdpa are not reloaded. should this

processor = AttnProcessor()

maybe become

AttnProcessor2_0() if hasattr(F, "scaled_dot_product_attention") else AttnProcessor()? @sayakpaul

AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* Add unload_ip_adapter method

* Update attn_processors with original layers

* Add test

* Use set_default_attn_processor

---------

Co-authored-by: Sayak Paul <[email protected]>
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.

[IP-Adapter] add a 'unload_ip_adapter' method

6 participants