[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-09 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22360 Merged to master/2.4 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-09 Thread mgaido91
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22360 Oh, right, it is already set in `copyValues`. Yes, then we can remove this (we should indeed since it takes no effect and can be misleading). Thanks, LGTM. --- ---

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-08 Thread WeichenXu123
Github user WeichenXu123 commented on the issue: https://github.com/apache/spark/pull/22360 Do we need to set `distanceMeasure` again for the parent model ? When parent model created, it will use the same `distanceMeasure` with the one used in training. --- -

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-08 Thread mgaido91
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22360 Yes, I think the point here is that the parameter is part of `BisectingKMeansParams` which defines as final the getter method. I think `KMeans` has the same issue. We can probably remove this and s

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-08 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22360 OK so the wrapper model's constructor needs to set this value once from the wrapped model? It does seem like it should be immutable and seems to not be exposed to set in k means for example. ---

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-08 Thread mgaido91
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22360 Thanks for pinging me @srowen. > But I don't see that its getDistanceMeasure would actually return the value inside that wrapped model? I am missing where the wrapper gets updated to retur

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22360 Oh right, the model, not the implementation. CC @mgaido91 too. OK we can take it out because it was only introduced for 2.4.0. Hm, so the model still has a distanceMeasure, but it's just not

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread WeichenXu123
Github user WeichenXu123 commented on the issue: https://github.com/apache/spark/pull/22360 @srowen The delegated `mllib.BisectingKMeansModel` is: ``` class BisectingKMeansModel private[clustering] ( private[clustering] val root: ClusteringTreeNode, @Since("2.4.0

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread srowen
Github user srowen commented on the issue: https://github.com/apache/spark/pull/22360 Pardon, why is it meaningless? It's used in the implementation it delegates to. --- - To unsubscribe, e-mail: reviews-unsubscr...

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22360 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22360 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95797/ Test PASSed. ---

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22360 **[Test build #95797 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95797/testReport)** for PR 22360 at commit [`64a1c27`](https://github.com/apache/spark/commit/6

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22360 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-unified/2929/

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22360 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #22360: [MINOR][ML] Remove `BisectingKMeansModel.setDistanceMeas...

2018-09-07 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22360 **[Test build #95797 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95797/testReport)** for PR 22360 at commit [`64a1c27`](https://github.com/apache/spark/commit/64