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

[SPARK-10353][MLLIB] BLAS gemm not scaling when beta = 0.0 for some subset of matrix multiplications #8525

Closed
wants to merge 2 commits into from

Conversation

brkyvz
Copy link
Contributor

@brkyvz brkyvz commented Aug 30, 2015

@mengxr @jkbradley @rxin

It would be great if this fix made it into RC3!

@brkyvz brkyvz changed the title [MLLIB] Fix BLAS not scaling with beta = 0.0 [SPARK-10353][MLLIB] BLAS gemm not scaling when beta = 0.0 for some subset of matrix multiplications Aug 30, 2015
@mengxr
Copy link
Contributor

mengxr commented Aug 30, 2015

@brkyvz Could you whether whether those branches are already implemented in f2jBLAS and reference BLAS? If they are already there, I think we should let them handle those special cases.

@brkyvz
Copy link
Contributor Author

brkyvz commented Aug 30, 2015

dgemm does handle the easy cases (alpha == 0.0 && beta == 1.0). dscal doesn't handle beta == 1.0. I still think we should keep them, because not all paths use BLAS

@brkyvz
Copy link
Contributor Author

brkyvz commented Aug 30, 2015

@mengxr
Copy link
Contributor

mengxr commented Aug 30, 2015

LGTM pending Jenkins.

@srowen
Copy link
Member

srowen commented Aug 30, 2015

+1 I thought we fixed this but it was something similar but not identical. I did not see similar occurrences.

@SparkQA
Copy link

SparkQA commented Aug 30, 2015

Test build #41796 has finished for PR 8525 at commit 4378ce8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 30, 2015

Test build #41802 has finished for PR 8525 at commit b535b53.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@asfgit asfgit closed this in 8d2ab75 Aug 30, 2015
asfgit pushed a commit that referenced this pull request Aug 30, 2015
…subset of matrix multiplications

mengxr jkbradley rxin

It would be great if this fix made it into RC3!

Author: Burak Yavuz <brkyvz@gmail.com>

Closes #8525 from brkyvz/blas-scaling.

(cherry picked from commit 8d2ab75)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
asfgit pushed a commit that referenced this pull request Aug 30, 2015
…subset of matrix multiplications

mengxr jkbradley rxin

It would be great if this fix made it into RC3!

Author: Burak Yavuz <brkyvz@gmail.com>

Closes #8525 from brkyvz/blas-scaling.

(cherry picked from commit 8d2ab75)
Signed-off-by: Xiangrui Meng <meng@databricks.com>
@mengxr
Copy link
Contributor

mengxr commented Aug 30, 2015

Merged into master, branch-1.5, and branch-1.4. @brkyvz Could you send another PR to fix branch-1.3? Thanks! It seems that on 1.3, we also have issues with if alpha == 0 return ....

srowen added a commit to srowen/spark that referenced this pull request Sep 2, 2015
asfgit pushed a commit that referenced this pull request Sep 2, 2015
…= 0.0 for some subset of matrix multiplications

Apply fixes for alpha, beta parameter handling in gemm/gemv from #8525 to branch 1.3

CC mengxr brkyvz

Author: Sean Owen <sowen@cloudera.com>

Closes #8572 from srowen/SPARK-10353.2.
@brkyvz brkyvz deleted the blas-scaling branch February 3, 2019 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants