Skip to content

[bugfix] bugfix for npu free memory #9640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 25, 2024
Merged

Conversation

leisuzz
Copy link
Contributor

@leisuzz leisuzz commented Oct 11, 2024

What does this PR do?

The original code will output error that torch_npu has no attribute empty_cache

Before submitting

Who can review?

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.

@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 11, 2024

@sayakpaul I split the PR into two as suggested, thanks for your help!

@sayakpaul
Copy link
Member

I am not sure if this PR is needed to allow NPU attention processor importing, do we?

@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 12, 2024

@sayakpaul After I added NPU attention, the computing speed / FPS actually increased

@sayakpaul
Copy link
Member

That is okay but I don't think we need to change anything to be able to import the NPU attention processor from diffusers.models.attention_processors no?

@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 12, 2024

@sayakpaul Thanks for your suggestion, I've added the NPU check condition to avoid unnecessary issues

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, left some comments. @yiyixuxu what do you think?

IPAdapterAttnProcessor2_0,
)
if is_torch_npu_available():
cross_attention_processors = (
Copy link
Member

Choose a reason for hiding this comment

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

These attention processors don’t depend on Torch NPU no?

IPAdapterAttnProcessor2_0,
)

attentionProcessor = Union[
Copy link
Member

Choose a reason for hiding this comment

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

We don’t follow camel casing in the diffusers codebase.

@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 14, 2024

@sayakpaul I've changed the code based on your suggestions, thanks!

@sayakpaul
Copy link
Member

Thank you but I am still struggling to understand #9640 (comment)

@leisuzz leisuzz changed the title Improve NPU performance [bugfix] bugfix for npu free memory Oct 24, 2024
@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 24, 2024

@sayakpaul Hello again, I changed the PR for bug fix because the original version tries to implement the wrong function for empty cache

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

@leisuzz
Copy link
Contributor Author

leisuzz commented Oct 25, 2024

@sayakpaul Seems like it needs a review for the requirement

@sayakpaul sayakpaul merged commit 94643fa into huggingface:main Oct 25, 2024
15 checks passed
@sayakpaul
Copy link
Member

Thanks!

sayakpaul added a commit that referenced this pull request Dec 23, 2024
* Improve NPU performance

* Improve NPU performance

* Improve NPU performance

* Improve NPU performance

* [bugfix] bugfix for npu free memory

* [bugfix] bugfix for npu free memory

* [bugfix] bugfix for npu free memory

---------

Co-authored-by: 蒋硕 <jiangshuo9@h-partners.com>
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.

3 participants