You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zhengruifeng <gi...@git.apache.org> on 2018/01/05 10:27:22 UTC
[GitHub] spark pull request #20164: [SPARK-22971][ML] OneVsRestModel should use tempo...
GitHub user zhengruifeng opened a pull request:
https://github.com/apache/spark/pull/20164
[SPARK-22971][ML] OneVsRestModel should use temporary RawPredictionCol
## What changes were proposed in this pull request?
use temporary RawPredictionCol in `OneVsRestModel#transform`
## How was this patch tested?
existing tests and added tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhengruifeng/spark ovr_not_use_getRawPredictionCol
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20164.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 #20164
----
commit f155e1cc6b175ac06a5f2ab710d4c053b0776507
Author: Zheng RuiFeng <ru...@...>
Date: 2018-01-05T09:29:25Z
create pr
commit 9b0dcc69535b6731c9b6cdc0030c846c3352a5de
Author: Zheng RuiFeng <ru...@...>
Date: 2018-01-05T10:19:59Z
create pr
commit 6c567ffb02738346fc83e467752add0d00a42e07
Author: Zheng RuiFeng <ru...@...>
Date: 2018-01-05T10:26:16Z
add test
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #85721 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85721/testReport)** for PR 20164 at commit [`6c567ff`](https://github.com/apache/spark/commit/6c567ffb02738346fc83e467752add0d00a42e07).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #85721 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85721/testReport)** for PR 20164 at commit [`6c567ff`](https://github.com/apache/spark/commit/6c567ffb02738346fc83e467752add0d00a42e07).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86147/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/20164
This pr is out of date. So I will close it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86157 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86157/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/20164
Sorry, I haven't understood where is the issue in current master code. The models here should be `ClassificationModel` and will always have `rawPrediction` param and have default value "rawPrediction". So what's the case current master code encounter error ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/20164
@WeichenXu123 Yes, my concern is that it is confusing if the transform failure is caused by column conflict by a ‘invisible’ column.
@srowen Agree that it is not perfect if we alter the output's raw prediction column. I think it maybe better to revert `rawPrediction` of base models after transform, and I will have a try.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86157 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86157/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85721/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/20164
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20164
OK, the code in question uses the `rawPredictionCol` from the `models` it is given. Yes they'd have to be unique for this to make sense, because it adds those raw prediction columns to the output. Isn't this always true though? You can't have a pipeline where elements add a column that exist, or were previously added by the pipeline.
I'm missing why it's not a problem to alter the output's raw prediction column from what the user had specified in `models`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86147/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86151 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86151/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #87058 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87058/testReport)** for PR 20164 at commit [`f9a922f`](https://github.com/apache/spark/commit/f9a922f92b3bc8fcca7df3dde67a989985b95fdd).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/20164
@srowen Different from the base model (like LoR), OVR and OVRModel do not have param `rawPredictionCol`.
So if the input dataframe contains a column which has the same name as base model's `getRawPredictionCol`, then OVRModel can not transform the input.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86147 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86147/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86151/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20164: [SPARK-22971][ML] OneVsRestModel should use tempo...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/20164#discussion_r160020496
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
@@ -170,21 +170,24 @@ final class OneVsRestModel private[ml] (
newDataset.persist(StorageLevel.MEMORY_AND_DISK)
}
+ // temporary column to store intermediate raw prediction
+ val tmpRawPredictionColName = "mbc$tmpraw" + UUID.randomUUID().toString
--- End diff --
in other ml cases we are slightly more descriptive with the prefix text
https://github.com/apache/spark/blob/576c43fb4226e4efa12189b41c3bc862019862c6/mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala#L1050
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #87058 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87058/testReport)** for PR 20164 at commit [`f9a922f`](https://github.com/apache/spark/commit/f9a922f92b3bc8fcca7df3dde67a989985b95fdd).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #85822 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85822/testReport)** for PR 20164 at commit [`263c64d`](https://github.com/apache/spark/commit/263c64d3d4da8dd3b9dd3009ac6e8fdfe9153dd5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86157/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/582/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20164: [SPARK-22971][ML] OneVsRestModel should use tempo...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng closed the pull request at:
https://github.com/apache/spark/pull/20164
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by zhengruifeng <gi...@git.apache.org>.
Github user zhengruifeng commented on the issue:
https://github.com/apache/spark/pull/20164
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20164: [SPARK-22971][ML] OneVsRestModel should use tempo...
Posted by MLnick <gi...@git.apache.org>.
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/20164#discussion_r161535696
--- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
@@ -170,21 +170,24 @@ final class OneVsRestModel private[ml] (
newDataset.persist(StorageLevel.MEMORY_AND_DISK)
}
+ // temporary column to store intermediate raw prediction
+ val tmpRawPredictionColName = "rawPrediction_" + UUID.randomUUID().toString
+
// update the accumulator column with the result of prediction of models
val aggregatedDataset = models.zipWithIndex.foldLeft[DataFrame](newDataset) {
case (df, (model, index)) =>
- val rawPredictionCol = model.getRawPredictionCol
- val columns = origCols ++ List(col(rawPredictionCol), col(accColName))
+ val columns = origCols ++ List(col(tmpRawPredictionColName), col(accColName))
--- End diff --
This line doesn't need to be in the `foldLeft` block any longer?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85822/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/20164
Oh, do you mean if input df including a column named "rawPrediction", then it will be overwritten when it transformed by OVSModel ? Looks like reasonable.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #86151 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86151/testReport)** for PR 20164 at commit [`e44d764`](https://github.com/apache/spark/commit/e44d7647fe6596c70a28527f893bdbdcb373c190).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20164
**[Test build #85822 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85822/testReport)** for PR 20164 at commit [`263c64d`](https://github.com/apache/spark/commit/263c64d3d4da8dd3b9dd3009ac6e8fdfe9153dd5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/20164
@srowen yeah, all models exist this issue. Although, a little difference, for other models, it is very straightforward for user to call `setRawPrediction` to avoid overwrite the same name column. But for OVSModel, it is a little tricky (user have to call the wrapped classification model 's `setRawPrediction` method), user is hard to aware the root cause why existing column be overwritten. So change it to use temporary column name seems to be a better way.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87058/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20164
Sure but isn't this the case with any column that any model adds that already exists -- why is this different?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20164
I might be missing something, but the user sets the raw prediction col name. Now it's going to use a different name than what the user set.
Isn't it simply an error to apply a second model, specifying the same raw prediction col name? you have to use a different name or else rename/remove the original.
Just like any other col that the code will add, if the user asks for a col to be added that exists already it's an error. Right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20164: [SPARK-22971][ML] OneVsRestModel should use temporary Ra...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20164
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org