Check F2C BLAS for OpenBLAS and other vendors #143846
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/143846
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 172edad with merge base f79689b ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
This LGTM, thanks @isuruf. Each of those BLAS libraries can be compiled with two ABIs indeed, and this check is needed (as the linked conda-forge issue shows). MKL, Accelerate and Eigen do not need to be checked. The one I wasn't familiar with was NVPL, and I can't find in its docs so quickly if it comes in two ABIs - I guess you know, and either way the check can't hurt.
|
Yeah, I didn't know about NVPL so I kept the check there. |
|
@atalman can you review? |
|
ping on this |
|
hi @isuruf could you please describe the issue you are trying to address ? Do you have a link ? |
|
@atalman, I updated the PR description. |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
can you review this @atalman ? |
|
Ping on this |
|
I am also running into problems that this change would fix. For example, Can we get this approved and merged? |
|
@pytorchbot rebase -b viable/strict |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 6 jobs have failed, first few of them are: linux-aarch64 / linux-jammy-aarch64-py3.10 / test (default, 2, 3, linux.arm64.m8g.4xlarge), linux-aarch64 / linux-jammy-aarch64-py3.10 / test (default, 3, 3, linux.arm64.m7g.4xlarge), linux-aarch64 / linux-jammy-aarch64-py3.10 / test (default, 3, 3, linux.arm64.m8g.4xlarge), trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m2-15), trunk / macos-py3-arm64 / test (mps, 1, 1, macos-m1-13) Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
This issue came from conda-forge/pytorch-cpu-feedstock#180. MKL follows the F2C convention for returning single precision floats as doubles and uses the G77 convention for returning complex valued scalars. OpenBLAS does the opposite. There is a check for this already, but it's done only when the Generic BLAS vendor code path is used and this PR moves that code to `Dependencies.cmake` to make it work when the BLAS vendor is OpenBLAS and others Pull Request resolved: pytorch#143846 Approved by: https://github.com/rgommers, https://github.com/atalman
Namely
```
CMake Warning (dev) in cmake/Dependencies.cmake:
A logical block opening on the line
/Users/nshulga/git/pytorch/pytorch/cmake/Dependencies.cmake:261 (if)
closes on the line
/Users/nshulga/git/pytorch/pytorch/cmake/Dependencies.cmake:263 (endif)
with mis-matching arguments.
```
Introduced by #143846
Namely
```
CMake Warning (dev) in cmake/Dependencies.cmake:
A logical block opening on the line
/Users/nshulga/git/pytorch/pytorch/cmake/Dependencies.cmake:261 (if)
closes on the line
/Users/nshulga/git/pytorch/pytorch/cmake/Dependencies.cmake:263 (endif)
with mis-matching arguments.
```
Introduced by #143846
Pull Request resolved: #159702
Approved by: https://github.com/cyyever, https://github.com/Skylion007
6 participants
Stack from ghstack (oldest at bottom):
This issue came from conda-forge/pytorch-cpu-feedstock#180. MKL follows the F2C convention for returning single precision floats as doubles and uses the G77 convention for returning complex valued scalars. OpenBLAS does the opposite. There is a check for this already, but it's done only when the Generic BLAS vendor code path is used and this PR moves that code to
Dependencies.cmaketo make it work when the BLAS vendor is OpenBLAS and others