Skip to content

Pass WITH_BLAS option from environment to CMake#78037

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

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

Conversation

@Flamefire
Copy link
Collaborator

@Flamefire Flamefire commented May 21, 2022

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

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

facebook-github-bot commented May 21, 2022 •
edited
Loading

🔗 Helpful links

✅ No Failures (0 Pending)

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

Expand to see more

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

This comment was automatically generated by Dr. CI (expand for details).

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

Click here to manually regenerate this comment.

@Flamefire Flamefire mentioned this pull request May 21, 2022
Closed
@mrshenli mrshenli requested review from malfet and seemethere May 22, 2022 22:19
@mrshenli mrshenli added module: build Build system issues triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels May 22, 2022
@janeyx99
Copy link
Contributor

janeyx99 commented May 23, 2022

@pytorchbot merge -g

@adamjstewart
Copy link
Contributor

adamjstewart commented May 25, 2022

How long does pytorchbot usually take to merge? Is this normal?

@adamjstewart adamjstewart mentioned this pull request May 25, 2022
Merged
@janeyx99
Copy link
Contributor

janeyx99 commented May 25, 2022

@pytorchbot merge -g

@janeyx99
Copy link
Contributor

janeyx99 commented May 25, 2022

@adamjstewart thanks for your perseverance in this matter--that is not normal. We experienced some cancellations the past week and the merge job must have been cancelled :/

@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2022

Hey @Flamefire.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@Flamefire Flamefire deleted the add_blas_opt branch May 25, 2022 13:40
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

@seemethere seemethere Awaiting requested review from seemethere

@malfet malfet Awaiting requested review from malfet

+1 more reviewer

@adamjstewart adamjstewart adamjstewart approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla signed Merged module: build Build system issues open source 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 @janeyx99 @adamjstewart @mrshenli @pytorchbot @pytorchmergebot