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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Torchscriptability FTW #252

Merged
merged 1 commit into from
Mar 24, 2022
Merged

[fix] Torchscriptability FTW #252

merged 1 commit into from
Mar 24, 2022

Conversation

blefaudeux
Copy link
Contributor

What does this PR do?

Fixes #246

Before submitting

  • [馃檭] Did you have fun?
  • Did you read the contributor guideline?
  • Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  • [o] Did you make sure to update the docs?
  • [o] Did you write any new necessary tests?
    • there are tests existing for torchscript, but not on the whole model. Some parts will not work for sure (triton for instance).
  • Did you update the changelog? (if needed)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 24, 2022
@blefaudeux
Copy link
Contributor Author

cc @jramapuram

@@ -125,7 +125,6 @@ def shape(self):
return self.values.shape

def __add__(self, other):
assert isinstance(other, type(self))
return AttentionMask(self.values + other.values, is_causal=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

follow the "if it looks like a duck and quack like a duck then it's a duck" python mantra. In any case @fmassa is rewriting this and it will disappear

@blefaudeux blefaudeux requested a review from fmassa March 24, 2022 17:55
@codecov-commenter
Copy link

Codecov Report

Merging #252 (490d4e7) into main (81eb650) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
- Coverage   92.21%   92.21%   -0.01%     
==========================================
  Files          60       60              
  Lines        3313     3312       -1     
==========================================
- Hits         3055     3054       -1     
  Misses        258      258              
Flag Coverage 螖
Python 92.21% <酶> (-0.01%) 猬囷笍

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage 螖
xformers/components/attention/attention_mask.py 98.48% <酶> (-0.03%) 猬囷笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 81eb650...490d4e7. Read the comment docs.

@blefaudeux blefaudeux merged commit 286b39e into main Mar 24, 2022
@blefaudeux blefaudeux deleted the fix_246 branch March 24, 2022 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT scripting is broken
4 participants