Skip to content

Pass WITH_BLAS option from environment to CMake#59220

Closed
Flamefire wants to merge 1 commit intopytorch:masterfrom
Flamefire:add_blas_opt
Closed

Pass WITH_BLAS option from environment to CMake#59220
Flamefire wants to merge 1 commit intopytorch:masterfrom
Flamefire:add_blas_opt

Conversation

@Flamefire
Copy link
Collaborator

@Flamefire Flamefire commented May 31, 2021

Allows to choose the BLAS backend with Eigen. Previously this was a CMake option only and the env variable was ignored.

Related to f1f3c8b

The claimed options BLAS=BLIS WITH_BLAS=blis are misleading: When BLAS=BLIS is set the WITH_BLAS option does not matter at all, it would only matter for BLAS=Eigen hence this issue went undetected so far.

@Flamefire
Pass WITH_BLAS option from environment to CMake
809f365 
Allows to choose the BLAS backend with Eigen
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 31, 2021 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 809f365 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@ngimel ngimel requested a review from janeyx99 June 1, 2021 20:33
@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 1, 2021
@cdeepali cdeepali mentioned this pull request Jun 23, 2021
Closed
@adamjstewart
Copy link
Contributor

adamjstewart commented Jun 25, 2021

Fixes #60328

@mohammadryousefi mohammadryousefi mentioned this pull request Feb 25, 2022
Closed
4 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2022

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Apr 13, 2022
@adamjstewart
Copy link
Contributor

adamjstewart commented Apr 13, 2022

@cdeepali (or anyone with maintainer privileges), can this PR be merged? We've been adding this patch to our Spack build recipe for PyTorch for the last year and haven't encountered any issues with it. This PR fixes #60328.

@janeyx99
Copy link
Contributor

janeyx99 commented Apr 13, 2022

oh no i had missed this a year ago!

@janeyx99
Copy link
Contributor

janeyx99 commented Apr 13, 2022

@pytorchbot merge this please!

@janeyx99
Copy link
Contributor

janeyx99 commented Apr 13, 2022

@adamjstewart Sorry about that--please don't hesitate to ping me (the designated reviewer) if I have not responded within 1-2 business days next time.

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Apr 13, 2022

Merge failed due to Command git -C /home/runner/work/pytorch/pytorch push origin master returned non-zero exit code 1
To https://github.com/pytorch/pytorch
! [rejected] master -> master (non-fast-forward)
error: failed to push some refs to 'https://github.com/pytorch/pytorch'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

Raised by https://github.com/pytorch/pytorch/actions/runs/2163560943

@janeyx99
Copy link
Contributor

janeyx99 commented Apr 13, 2022

Oh, it looks like it's too old to be merged. Can you rebase locally and repush and I'll merge it then?

@github-actions github-actions bot closed this May 13, 2022
@adamjstewart
Copy link
Contributor

adamjstewart commented May 21, 2022

@Flamefire are you able to rebase this PR on master? If not I can push a similar PR. Would be good to see this merged.

@Flamefire Flamefire mentioned this pull request May 21, 2022
Closed
@Flamefire
Copy link
Collaborator Author

Flamefire commented May 21, 2022

@adamjstewart Thanks for the ping. Rebased this but it seems it doesn't show up in this PR as it got already closed

@janeyx99 I opened a new PR #78037 with the same branch and copied the description. This still applies so please merge soon to avoid further conflicts.

pytorchmergebot pushed a commit that referenced this pull request May 25, 2022
@Flamefire @pytorchmergebot
Pass WITH_BLAS option from environment to CMake (#78037)
d5f9958 
Allows to choose the BLAS backend with Eigen. Previously this was a CMake option only and the env variable was ignored.

Related to f1f3c8b

The claimed options BLAS=BLIS WITH_BLAS=blis are misleading: When BLAS=BLIS is set the WITH_BLAS option does not matter at all, it would only matter for BLAS=Eigen hence this issue went undetected so far.

Supersedes #59220
Pull Request resolved: #78037
Approved by: https://github.com/adamjstewart, https://github.com/janeyx99
facebook-github-bot pushed a commit that referenced this pull request May 26, 2022
@Flamefire @facebook-github-bot
Pass WITH_BLAS option from environment to CMake (#78037) (#78037)
f555fa8 
Summary:
Allows to choose the BLAS backend with Eigen. Previously this was a CMake option only and the env variable was ignored.

Related to f1f3c8b

The claimed options BLAS=BLIS WITH_BLAS=blis are misleading: When BLAS=BLIS is set the WITH_BLAS option does not matter at all, it would only matter for BLAS=Eigen hence this issue went undetected so far.

Supersedes #59220

Pull Request resolved: #78037
Approved by: https://github.com/adamjstewart, https://github.com/janeyx99

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/d5f99581b508df04edecbd0be10312b4b6d5ae03

Reviewed By: mehtanirav

Differential Revision: D36668892

fbshipit-source-id: 659d2168dd0c3372776ae82616554d0799c91cf9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@janeyx99 janeyx99 janeyx99 approved these changes

Assignees

No one assigned

Labels

cla signed open source Stale triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Flamefire @facebook-github-bot @adamjstewart @janeyx99 @pytorchmergebot @ngimel @pytorchbot