You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hujy <gi...@git.apache.org> on 2016/04/19 06:16:32 UTC
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
GitHub user hujy opened a pull request:
https://github.com/apache/spark/pull/12491
[SPARK-14712][ML]spark.ml.LogisticRegressionModel.toString should sum…
## What changes were proposed in this pull request?
[SPARK-14712](https://issues.apache.org/jira/browse/SPARK-14712)
spark.mllib LogisticRegressionModel overrides toString to print a little model info. We should do the same in spark.ml and override _repr_ in pyspark.
## How was this patch tested?
LogisticRegressionSuite.scala
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hujy/spark 14712
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12491.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12491
----
commit 4f2b3e0948e6beb2ca7591d1b5719046e68e857f
Author: HuJiayin <ji...@intel.com>
Date: 2016-04-19T04:11:09Z
[SPARK-14712][ML]spark.ml.LogisticRegressionModel.toString should summarize model
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60175230
--- Diff: python/pyspark/mllib/classification.py ---
@@ -262,6 +262,8 @@ def load(cls, sc, path):
model.setThreshold(threshold)
return model
+ def __repr__(self):
+ return self._call_java("toString")
--- End diff --
ok :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the pull request:
https://github.com/apache/spark/pull/12491#issuecomment-211729265
Thanks for taking the initiative on this PR - at first glance it seems like this approach might not quite work but its easier to tell with some tests - could you add a test case and run it locally? You add your test in LogisticRegressionSuite.scala for the scala test. As well you may find the linter tools ./dev/lint-scala & ./dev/lint-python to be useful :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/12491
To get this tested we will need a committer to say so. Maybe @jkbradley who created the JIRA can take a look / add you to the whitelist.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12491
**[Test build #3520 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3520/testReport)** for PR 12491 at commit [`2dbe552`](https://github.com/apache/spark/commit/2dbe552ea86386ece1d4f0292cb8f0c42f6df503).
* This patch **fails Python style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r82482033
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -625,6 +625,13 @@ class LogisticRegressionModel private[spark] (
*/
@Since("1.6.0")
override def write: MLWriter = new LogisticRegressionModel.LogisticRegressionModelWriter(this)
+
+ @Since("2.0.0")
--- End diff --
We will need to update this since :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60172784
--- Diff: python/pyspark/mllib/classification.py ---
@@ -262,6 +262,8 @@ def load(cls, sc, path):
model.setThreshold(threshold)
return model
+ def __repr__(self):
+ return self._call_java("toString")
--- End diff --
I think Python style asks for two new lines here (try running ./dev/lint-python locally :))
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by bravo-zhang <gi...@git.apache.org>.
Github user bravo-zhang commented on the issue:
https://github.com/apache/spark/pull/12491
Hi @holdenk , to continue this PR I opened https://github.com/apache/spark/pull/18826
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r82482185
--- Diff: python/pyspark/ml/classification.py ---
@@ -273,6 +273,10 @@ def evaluate(self, dataset):
java_blr_summary = self._call_java("evaluate", dataset)
return BinaryLogisticRegressionSummary(java_blr_summary)
+ @since("2.0.0")
+ def __repr__(self):
--- End diff --
It might make sense to include this in the docstring since it might be useful for people to see how to use it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60175227
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -159,7 +159,9 @@ private[classification] trait LogisticRegressionParams extends ProbabilisticClas
@Since("1.2.0")
@Experimental
class LogisticRegression @Since("1.2.0") (
- @Since("1.4.0") override val uid: String)
+ @Since("1.4.0") override val uid: String,
+ @Since("2.0.0") val numFeatures: Int = 0,
--- End diff --
I think the values are passed in when user create the object. When user call toString, the values are returned.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12491#issuecomment-211722740
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on the pull request:
https://github.com/apache/spark/pull/12491#issuecomment-218922699
ok with test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on the issue:
https://github.com/apache/spark/pull/12491
We can also try reaching out to @davies who does a bunch of Python related reviews. @davies can you give Jenkins the OK to run the tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/12491
**[Test build #3520 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3520/testReport)** for PR 12491 at commit [`2dbe552`](https://github.com/apache/spark/commit/2dbe552ea86386ece1d4f0292cb8f0c42f6df503).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12491
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60182574
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -23,7 +23,7 @@ import breeze.linalg.{DenseVector => BDV}
import breeze.optimize.{CachedDiffFunction, DiffFunction, LBFGS => BreezeLBFGS, OWLQN => BreezeOWLQN}
import org.apache.hadoop.fs.Path
-import org.apache.spark.SparkException
+import org.apache.spark.{SparkException}
--- End diff --
This change seems unneeded.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on the pull request:
https://github.com/apache/spark/pull/12491#issuecomment-218650789
@jkbradley, I tested the patch and included the new unit test cases. Please have a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60247254
--- Diff: python/pyspark/ml/classification.py ---
@@ -123,7 +123,8 @@ def __init__(self, featuresCol="features", labelCol="label", predictionCol="pred
def setParams(self, featuresCol="features", labelCol="label", predictionCol="prediction",
maxIter=100, regParam=0.0, elasticNetParam=0.0, tol=1e-6, fitIntercept=True,
threshold=0.5, thresholds=None, probabilityCol="probability",
- rawPredictionCol="rawPrediction", standardization=True, weightCol=None):
+ rawPredictionCol="rawPrediction", standardization=True, weightCol=None,
+ numFeatures=0, numClasses=0):
--- End diff --
```numFeatures``` and ```numClasses``` should not params which can be set by users.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by hujy <gi...@git.apache.org>.
Github user hujy commented on the pull request:
https://github.com/apache/spark/pull/12491#issuecomment-211766721
@holdenk, I move the toString to ml LogisticRegresstionModel and remove the new input parameters to LogisticRegression. I added the test case in LogisticRegressionSuite.scala : )
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/12491
@hujy Sorry for the delay!
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #12491: [SPARK-14712][ML]spark.ml.LogisticRegressionModel.toStri...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/12491
@hujy Would you fix the test failure? If it is inactive, I would rather like to suggest to close this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14712][ML]spark.ml.LogisticRegressionMo...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/12491#discussion_r60173022
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -159,7 +159,9 @@ private[classification] trait LogisticRegressionParams extends ProbabilisticClas
@Since("1.2.0")
@Experimental
class LogisticRegression @Since("1.2.0") (
- @Since("1.4.0") override val uid: String)
+ @Since("1.4.0") override val uid: String,
+ @Since("2.0.0") val numFeatures: Int = 0,
--- End diff --
Why are we adding these vals here? Where do they get set from? Are they needed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org