-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-11605] [MLlib] ML 1.6 QA: API: Java compatibility, docs #10102
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
Conversation
|
Test build #47060 has finished for PR 10102 at commit
|
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.
This can be Int to support multiClass sample data, right?
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.
This was not part of the design, but I agree it would be nice someday. I'll ping @mengxr since he reviewed the original PRs, but I think we'll keep it as is for now.
|
I'll take a look now |
|
I wouldn't worry about the getThreshold issue. I haven't seen users complain about it, and they will hopefully switch over to the spark.ml API anyways. Can you please modify the old registerStream taking tuples to use BinarySample as well? We should be consistent across Scala and Java. |
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.
organize imports
|
Test build #47316 has finished for PR 10102 at commit
|
|
hudson.plugins.git.GitException: Failed to fetch from https://github.com/apache/spark.git need a retest |
|
@jkbradley Thanks for the review. Updated. |
|
Thank you for updating! LGTM pending tests. |
|
Test build #2185 has finished for PR 10102 at commit
|
|
Merging with master and branch-1.6 |
jira: https://issues.apache.org/jira/browse/SPARK-11605 Check Java compatibility for MLlib for this release. fix: 1. `StreamingTest.registerStream` needs java friendly interface. 2. `GradientBoostedTreesModel.computeInitialPredictionAndError` and `GradientBoostedTreesModel.updatePredictionError` has java compatibility issue. Mark them as `developerAPI`. TBD: [updated] no fix for now per discussion. `org.apache.spark.mllib.classification.LogisticRegressionModel` `public scala.Option<java.lang.Object> getThreshold();` has wrong return type for Java invocation. `SVMModel` has the similar issue. Yet adding a `scala.Option<java.util.Double> getThreshold()` would result in an overloading error due to the same function signature. And adding a new function with different name seems to be not necessary. cc jkbradley feynmanliang Author: Yuhao Yang <hhbyyh@gmail.com> Closes #10102 from hhbyyh/javaAPI. (cherry picked from commit 5cb4695) Signed-off-by: Joseph K. Bradley <joseph@databricks.com>
jira: https://issues.apache.org/jira/browse/SPARK-11605
Check Java compatibility for MLlib for this release.
fix:
StreamingTest.registerStreamneeds java friendly interface.GradientBoostedTreesModel.computeInitialPredictionAndErrorandGradientBoostedTreesModel.updatePredictionErrorhas java compatibility issue. Mark them asdeveloperAPI.TBD:
[updated] no fix for now per discussion.
org.apache.spark.mllib.classification.LogisticRegressionModelpublic scala.Option<java.lang.Object> getThreshold();has wrong return type for Java invocation.SVMModelhas the similar issue.Yet adding a
scala.Option<java.util.Double> getThreshold()would result in an overloading error due to the same function signature. And adding a new function with different name seems to be not necessary.cc @jkbradley @feynmanliang