Skip to content

[ExecuTorch][Llama] Use Eigen blas for custom sdpa #6229

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

Closed
wants to merge 2 commits into from

Conversation

kimishpatel
Copy link
Contributor

@kimishpatel kimishpatel commented Oct 15, 2024

Stack from ghstack (oldest at bottom):

OpenBlas's implementation is not thread safe. Thus when used within
parallel_for, it produces incorrect output. This has been documented in a few
places like here OpenMathLib/OpenBLAS#1441 and
OpenMathLib/OpenBLAS#2543.

I tried few options to disable openblas's multithreading but none of them
seemed to work.

It is possible that upstream openblas has fixed this. Will validate this by
pulling in latest, but in the meanwhile using eigen_blas to unblock.

Differential Revision: D64398816

OpenBlas's implementation is not thread safe. Thus when used within
parallel_for, it produces incorrect output. This has been documented in a few
places like here OpenMathLib/OpenBLAS#1441 and
OpenMathLib/OpenBLAS#2543.

I tried few options to disable openblas's multithreading but none of them
seemed to work.

It is possible that upstream openblas has fixed this. Will validate this by
pulling in latest, but in the meanwhile using eigen_blas to unblock.

Differential Revision: [D64398816](https://our.internmc.facebook.com/intern/diff/D64398816/)

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Oct 15, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6229

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit e74aab8 with merge base 7ba7990 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@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 Oct 15, 2024
kimishpatel added a commit that referenced this pull request Oct 15, 2024
OpenBlas's implementation is not thread safe. Thus when used within
parallel_for, it produces incorrect output. This has been documented in a few
places like here OpenMathLib/OpenBLAS#1441 and
OpenMathLib/OpenBLAS#2543.

I tried few options to disable openblas's multithreading but none of them
seemed to work.

It is possible that upstream openblas has fixed this. Will validate this by
pulling in latest, but in the meanwhile using eigen_blas to unblock.

Differential Revision: [D64398816](https://our.internmc.facebook.com/intern/diff/D64398816/)

ghstack-source-id: 248080389
Pull Request resolved: #6229
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64398816

OpenBlas's implementation is not thread safe. Thus when used within
parallel_for, it produces incorrect output. This has been documented in a few
places like here OpenMathLib/OpenBLAS#1441 and
OpenMathLib/OpenBLAS#2543.

I tried few options to disable openblas's multithreading but none of them
seemed to work.

It is possible that upstream openblas has fixed this. Will validate this by
pulling in latest, but in the meanwhile using eigen_blas to unblock.

Differential Revision: [D64398816](https://our.internmc.facebook.com/intern/diff/D64398816/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64398816

kimishpatel added a commit that referenced this pull request Oct 15, 2024
Pull Request resolved: #6229

OpenBlas's implementation is not thread safe. Thus when used within
parallel_for, it produces incorrect output. This has been documented in a few
places like here OpenMathLib/OpenBLAS#1441 and
OpenMathLib/OpenBLAS#2543.

I tried few options to disable openblas's multithreading but none of them
seemed to work.

It is possible that upstream openblas has fixed this. Will validate this by
pulling in latest, but in the meanwhile using eigen_blas to unblock.
ghstack-source-id: 248095356
@exported-using-ghexport

Differential Revision: [D64398816](https://our.internmc.facebook.com/intern/diff/D64398816/)
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 97a1965.

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. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants