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

Correct NATTEN function signatures and force new version #22298

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

alihassanijr
Copy link
Contributor

What does this PR do?

This complements #22229. (Sorry for breaking it into two; I was traveling when I realized the issue so I only started a quick PR to fix circleci builds.)

We're releasing a new NATTEN build that corrects the signature inconsistency between the function calls (see my comment in the previous PR for more.)

Rather than wait for a future build, we decided to do it right now because we could end up forgetting to open a PR to transformers.

We're finishing up testing the new build, but I figured I'd open this PR before I push to PyPI. If circleci tries to get the latest version from PyPI it would fail the unit tests associated with models depending on this package.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@ydshieh @amyeroberts @sgugger

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 21, 2023

The documentation is not available anymore as the PR was closed or merged.

@alihassanijr
Copy link
Contributor Author

Unsure why a Flax test is failing. I'm assuming I'll have to wait for another PR to merge and then rebase?

@ydshieh
Copy link
Collaborator

ydshieh commented Mar 21, 2023

Hi, Thank you for the fix and PR. Regarding failing flax test, you can ignore it🤗

@sgugger
Copy link
Collaborator

sgugger commented Mar 21, 2023

This is needed to fix the CI on the main branch (broken by the new release of natten) so merging.

@sgugger sgugger merged commit 5990743 into huggingface:main Mar 21, 2023
@alihassanijr alihassanijr deleted the fix_natten_sig branch March 21, 2023 21:25
raghavanone pushed a commit to raghavanone/transformers that referenced this pull request Apr 5, 2023
alihassanijr added a commit to alihassanijr/StyleNAT that referenced this pull request Apr 5, 2023
NATTEN functions take in kernel size as well as dilation since 0.14.6.
The change in signature breaks Hydra-NA, which calls those functions
directly instead of using the nn.Module.

This fixes SHI-Labs#8 .

Refs:

* https://github.com/SHI-Labs/NATTEN/releases/tag/v0.14.6
* https://github.com/SHI-Labs/NATTEN/blob/main/CHANGELOG.md#0146---2023-03-21

Same change in other repositories using NATTEN:
* huggingface/transformers#22229
* huggingface/transformers#22298
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
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.

None yet

4 participants