You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jkbradley <gi...@git.apache.org> on 2016/03/24 05:42:57 UTC
[GitHub] spark pull request: [SPARK-10691][ML] Make LogisticRegressionModel...
GitHub user jkbradley opened a pull request:
https://github.com/apache/spark/pull/11928
[SPARK-10691][ML] Make LogisticRegressionModel, LinearRegressionModel evaluate() public
## What changes were proposed in this pull request?
Made evaluate method public. Fixed LogisticRegressionModel evaluate to handle case when probabilityCol is not specified.
## How was this patch tested?
There were already unit tests for these methods.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jkbradley/spark public-evaluate
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11928.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 #11928
----
commit 836ec920108646481c9f11ff4426c9efdd4c7df9
Author: Joseph K. Bradley <jo...@databricks.com>
Date: 2016-03-24T04:40:59Z
Made LogisticRegression, LinearRegression evaluate() public
----
---
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-10691][ML] Make LogisticRegressionModel...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-202193153
I'll merge this with master
---
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-10691][ML] Make LogisticRegressionModel...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201529661
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54213/
Test PASSed.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:
https://github.com/apache/spark/pull/11928#discussion_r57490499
--- Diff: mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala ---
@@ -412,11 +412,10 @@ class LinearRegressionModel private[ml] (
def hasSummary: Boolean = trainingSummary.isDefined
/**
- * Evaluates the model on a testset.
+ * Evaluates the model on a test dataset.
* @param dataset Test dataset to evaluate model on.
*/
- // TODO: decide on a good name before exposing to public API
- private[regression] def evaluate(dataset: DataFrame): LinearRegressionSummary = {
+ def evaluate(dataset: DataFrame): LinearRegressionSummary = {
// Handle possible missing or invalid prediction columns
val (summaryModel, predictionColName) = findSummaryModelAndPredictionCol()
new LinearRegressionSummary(summaryModel.transform(dataset), predictionColName,
--- End diff --
You're right; it should be summaryModel. It is possible for users to turn off the predictionCol and just output other cols, but it is an obscure use case.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201500671
**[Test build #54213 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54213/consoleFull)** for PR 11928 at commit [`fdce26f`](https://github.com/apache/spark/commit/fdce26f1fe82a0ae5cf740afc36b3edc1004a321).
---
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-10691][ML] Make LogisticRegressionModel...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on a diff in the pull request:
https://github.com/apache/spark/pull/11928#discussion_r57384219
--- Diff: mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala ---
@@ -412,11 +412,10 @@ class LinearRegressionModel private[ml] (
def hasSummary: Boolean = trainingSummary.isDefined
/**
- * Evaluates the model on a testset.
+ * Evaluates the model on a test dataset.
* @param dataset Test dataset to evaluate model on.
--- End diff --
same as above `@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-10691][ML] Make LogisticRegressionModel...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201529245
**[Test build #54213 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54213/consoleFull)** for PR 11928 at commit [`fdce26f`](https://github.com/apache/spark/commit/fdce26f1fe82a0ae5cf740afc36b3edc1004a321).
* This patch passes all 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: [SPARK-10691][ML] Make LogisticRegressionModel...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-200676206
Merged build finished. Test PASSed.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201529656
Merged build finished. Test PASSed.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-200675840
**[Test build #54000 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54000/consoleFull)** for PR 11928 at commit [`836ec92`](https://github.com/apache/spark/commit/836ec920108646481c9f11ff4426c9efdd4c7df9).
* This patch passes all 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: [SPARK-10691][ML] Make LogisticRegressionModel...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201015402
Thanks for getting to this @jkbradley. I had just a few comments, otherwise LGTM. I'll update my PR with evaluate after this goes in.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-200989543
@BryanCutler Would you mind taking a look at this PR to help review it? Thanks!
---
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-10691][ML] Make LogisticRegressionModel...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-200676208
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54000/
Test PASSed.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on a diff in the pull request:
https://github.com/apache/spark/pull/11928#discussion_r57384160
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -539,13 +539,14 @@ class LogisticRegressionModel private[spark] (
def hasSummary: Boolean = trainingSummary.isDefined
/**
- * Evaluates the model on a testset.
+ * Evaluates the model on a test dataset.
* @param dataset Test dataset to evaluate model on.
*/
--- End diff --
I think we would want a `@since(2.0.0)` now that it is public right?
---
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-10691][ML] Make LogisticRegressionModel...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on a diff in the pull request:
https://github.com/apache/spark/pull/11928#discussion_r57385094
--- Diff: mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala ---
@@ -412,11 +412,10 @@ class LinearRegressionModel private[ml] (
def hasSummary: Boolean = trainingSummary.isDefined
/**
- * Evaluates the model on a testset.
+ * Evaluates the model on a test dataset.
* @param dataset Test dataset to evaluate model on.
*/
- // TODO: decide on a good name before exposing to public API
- private[regression] def evaluate(dataset: DataFrame): LinearRegressionSummary = {
+ def evaluate(dataset: DataFrame): LinearRegressionSummary = {
// Handle possible missing or invalid prediction columns
val (summaryModel, predictionColName) = findSummaryModelAndPredictionCol()
new LinearRegressionSummary(summaryModel.transform(dataset), predictionColName,
--- End diff --
I think that `summaryModel` should be passed into the `LinearRegressionSummary` constructor instead of `this`. I think I see the predictionCol used in `devianceResiduals`, so if it was just added in the `summaryModel`, it might fail.
---
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-10691][ML] Make LogisticRegressionModel...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-200662362
**[Test build #54000 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54000/consoleFull)** for PR 11928 at commit [`836ec92`](https://github.com/apache/spark/commit/836ec920108646481c9f11ff4426c9efdd4c7df9).
---
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-10691][ML] Make LogisticRegressionModel...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on a diff in the pull request:
https://github.com/apache/spark/pull/11928#discussion_r57383969
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
@@ -539,13 +539,14 @@ class LogisticRegressionModel private[spark] (
def hasSummary: Boolean = trainingSummary.isDefined
/**
- * Evaluates the model on a testset.
+ * Evaluates the model on a test dataset.
* @param dataset Test dataset to evaluate model on.
*/
- // TODO: decide on a good name before exposing to public API
- private[classification] def evaluate(dataset: DataFrame): LogisticRegressionSummary = {
- new BinaryLogisticRegressionSummary(
- this.transform(dataset), $(probabilityCol), $(labelCol), $(featuresCol))
+ def evaluate(dataset: DataFrame): LogisticRegressionSummary = {
+ // Handle possible missing or invalid prediction columns
+ val (summaryModel, probabilityColName) = this.findSummaryModelAndProbabilityCol()
--- End diff --
nit: using `this` isn't necessary
---
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-10691][ML] Make LogisticRegressionModel...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11928
---
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-10691][ML] Make LogisticRegressionModel...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:
https://github.com/apache/spark/pull/11928#issuecomment-201499502
@BryanCutler Thanks for the review!
---
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