Skip to content
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

Deleted the 'maybe_free_model_hooks()' from Diffusers Pipelines #330

Merged

Conversation

Cerrix
Copy link
Contributor

@Cerrix Cerrix commented Nov 16, 2023

As described in: #327 issue the maybe_free_model_hooks() present in the following 3 pipelines:

is removed.
This will allow the pipelines to work as expected.

…icable to AWS Neuron and - for this reason - origin of an error in the pipelines
@Cerrix
Copy link
Contributor Author

Cerrix commented Nov 16, 2023

@JingyaHuang here the PR 😄

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@JingyaHuang JingyaHuang self-requested a review November 16, 2023 15:47
Copy link
Collaborator

@JingyaHuang JingyaHuang left a comment

Choose a reason for hiding this comment

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

LGTM, thanks again for reporting the issue and contributing! Let's wait a bit for the inf2 CI to complete before merging.

@Cerrix
Copy link
Contributor Author

Cerrix commented Nov 16, 2023

Yep! But it failed 🥲 I don’t understand why, it does not seem related to my commit

@JingyaHuang
Copy link
Collaborator

@Cerrix No worries, it failed because of some access issues.

It seems that after the refactoring of CIs, PRs from contributors not in the org won't have access to HF_TOKEN_OPTIMUM_NEURON_CI leading to the failure of some tests on hub integration. Do you know why? @dacorvo, the easiest workaround will be just add decorators like requires_hf_token on those tests. But it would be better if we can have contributors's PRs run those tests as well.

cc. @michaelbenayoun

@michaelbenayoun
Copy link
Member

Yes, seeing the same thing for #332. Let's discuss it offline on how we could address this.

@JingyaHuang
Copy link
Collaborator

I will merge this PR as failed tests are irrelevant and I would like to have it in today's release. cc. @dacorvo @michaelbenayoun

@JingyaHuang JingyaHuang merged commit d00d709 into huggingface:main Nov 17, 2023
4 of 7 checks passed
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.

4 participants