-
Notifications
You must be signed in to change notification settings - Fork 27.9k
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
Fix natten #22229
Fix natten #22229
Conversation
The new NATTEN 0.14.5 supports PyTorch 2.0, but also adds an additional argument to the QK operation to allow optional RPBs. This ends up failing NATTEN tests. This commit adds NATTEN back to circleci and adds the arguments to get it working again.
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the (super fast) fix ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes!
@alihassanijr Thanks for such a quick fix! Just double checking - is this version of |
My pleasure, sorry I didn't realize this earlier. So the problem was that we had a pull request a couple of months ago that added an additional argument to one of the C functions. We didn't immediately rebuild and push out a new release at the time. On a different note, we only had to change this here in the first place because we explicitly use the C function calls in the models using NA: We could try and figure out how we would directly import the nn.Module we typically encourage everyone to use, that way any changes to the signatures would not affect NATTEN can in theory support future PyTorch versions without any change required (unless PyTorch changes anything in their ATEN backend like they did with the dispatchers in 1.13, which would require us to work those changes into our CPP backend as well.) I've been wanting to set up CircleCI or Travis so that I wouldn't have to set that up manually every time there's a new PyTorch release, but just haven't found the time to do so yet. But we will to the best of our abilities try to build them upon new PyTorch releases and push them out as soon as possible. |
* Add kernel size to NATTEN's QK arguments. The new NATTEN 0.14.5 supports PyTorch 2.0, but also adds an additional argument to the QK operation to allow optional RPBs. This ends up failing NATTEN tests. This commit adds NATTEN back to circleci and adds the arguments to get it working again. * Force NATTEN >= 0.14.5
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
* Add kernel size to NATTEN's QK arguments. The new NATTEN 0.14.5 supports PyTorch 2.0, but also adds an additional argument to the QK operation to allow optional RPBs. This ends up failing NATTEN tests. This commit adds NATTEN back to circleci and adds the arguments to get it working again. * Force NATTEN >= 0.14.5
What does this PR do?
The new NATTEN 0.14.5 supports PyTorch 2.0, but also adds an additional
argument to the QK operation to allow optional RPBs.
This ends up failing NATTEN tests.
This commit adds NATTEN back to circleci and adds the arguments to get
it working again.
Reverts #22218.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@sgugger
Misc
Related issues on NATTEN:
SHI-Labs/NATTEN#23
SHI-Labs/NATTEN#19