-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-6685][ML]Use DSYRK to compute AtA in ALS #19536
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
Changes from all commits
8fb4a82
7e3d238
3607bdc
56194eb
dc4f4ba
d29fd67
294164d
513e791
f56b586
1081e64
a6b5a16
2077457
061df75
4fdcbe0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1043,6 +1043,9 @@ object MimaExcludes { | |
| // [SPARK-21680][ML][MLLIB]optimzie Vector coompress | ||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.spark.mllib.linalg.Vector.toSparseWithSize"), | ||
| ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.spark.ml.linalg.Vector.toSparseWithSize") | ||
| ) ++ Seq( | ||
| // [SPARK-6685][ML]Use DSYRK to compute AtA in ALS | ||
| ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.ml.recommendation.ALS.train") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This breaks an API unnecessarily, even though it's a dev API. I think we instead need to remove this as a user-facing param and avoid it altogether.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am ok to remove this, and use a loose threshold (e.g. 100), which is helpful for most cases. How about it? |
||
| ) | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure callers can meaningfully understand and set this. Can't we pick a threshold programmatically?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, my test result is better than the previous result, especially for native BLAS. I will update my test results here soon, and I will change this set.