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

Enhance the robustness of the flash attention check #20495

Merged
merged 7 commits into from
Nov 15, 2024

Conversation

james77777778
Copy link
Contributor

Also fixes GPU CI.

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.06%. Comparing base (0861e09) to head (94f25ce).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
keras/src/backend/torch/nn.py 75.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #20495      +/-   ##
==========================================
- Coverage   82.09%   82.06%   -0.03%     
==========================================
  Files         515      515              
  Lines       47575    47615      +40     
  Branches     7463     8531    +1068     
==========================================
+ Hits        39056    39077      +21     
- Misses       6710     6721      +11     
- Partials     1809     1817       +8     
Flag Coverage Δ
keras 81.92% <82.35%> (-0.03%) ⬇️
keras-jax 64.99% <35.29%> (-0.03%) ⬇️
keras-numpy 59.95% <11.76%> (-0.03%) ⬇️
keras-tensorflow 65.98% <11.76%> (-0.03%) ⬇️
keras-torch 64.91% <58.82%> (-0.02%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@james77777778
Copy link
Contributor Author

james77777778 commented Nov 15, 2024

This PR should be ready to run on GPU CI again now. I’m wondering if it’s possible to use a newer GPU for Keras GPU CI, as flash attention isn't available on T4, and these tests are currently being skipped.

Colab: https://colab.research.google.com/drive/1-fQdyAs-w5lM7ZGN8mroWmQP9HBJxYcK?usp=sharing

EDIT:
This PR pins coverage==7.6.1 to overcome this issue: nedbat/coveragepy#1891

BTW, I saw the announcement. Good luck and best wishes, @fchollet!
I've really enjoyed collaborating with you, and I’m proud of my contributions to Keras. Looking forward to seeing your next move!

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM, tank you!

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Nov 15, 2024
@fchollet fchollet merged commit c014c5e into keras-team:master Nov 15, 2024
9 checks passed
@google-ml-butler google-ml-butler bot removed ready to pull Ready to be merged into the codebase kokoro:force-run labels Nov 15, 2024
@nedbat
Copy link

nedbat commented Nov 15, 2024

The assert in coverage.py 7.6.5 is now fixed and released as part of coverage 7.6.6.

@james77777778 james77777778 deleted the improve-mha branch November 15, 2024 14:14
@james77777778 james77777778 mentioned this pull request Nov 15, 2024
wang-xianghao pushed a commit to wang-xianghao/keras-dev that referenced this pull request Nov 20, 2024
* Enhance the robustness of the flash attention check.

* Fix CI

* Fix CI again

* Fix GPU CI again and again...

* No raise in tests

* Pin coverage==7.6.1

* Fix the comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants