[SPARK-3726] [MLlib] Allow sampling_rate not equal to 1.0 in RandomForests #4073

MechCoder · 2015-01-16T16:50:54Z

I've added support for sampling_rate not equal to 1.0 . I have two major questions.

A Scala style test is failing, since the number of parameters now exceed 10.
I would like suggestions to understand how to test this.

SparkQA · 2015-01-16T16:52:37Z

Test build #25666 has started for PR 4073 at commit 6685b44.

This patch merges cleanly.

MechCoder · 2015-01-16T16:52:41Z

@jkbradley @mengxr it would be great if you could have a look.

SparkQA · 2015-01-16T16:53:29Z

Test build #25666 has finished for PR 4073 at commit 6685b44.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-16T16:53:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25666/
Test FAILed.

MechCoder · 2015-01-16T18:21:36Z

I've made changes such that this not break anything, i.e everything is backward compat.

MechCoder · 2015-01-16T18:52:11Z

@jkbradley Oops, the comments got deleted somehow. I meant that this is because there are 10 arguments in trainClassifier and trainRegressor

jkbradley · 2015-01-16T18:57:10Z

@MechCoder Taking a closer look, I now realize that part of this functionality is already there...see the JIRA & let me know what you think.

MechCoder · 2015-01-16T19:03:51Z

Oh well, but still if I'm not mistaken, the subSamplingRate is overriden by the condition numTrees > 1. This should not be the case as having a lower sampling, might help in this case too (when using the entire data might be expensive, ofcourse assuming the sampling is a good representation of the data itself). IMO, That should be fixed, right?

SparkQA · 2015-01-16T19:07:42Z

Test build #25672 has started for PR 4073 at commit 6685b44.

This patch merges cleanly.

jkbradley · 2015-01-16T19:08:18Z

Good point, yes, I think it's worth fixing.

SparkQA · 2015-01-16T19:08:41Z

Test build #25672 has finished for PR 4073 at commit 6685b44.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-16T19:08:42Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25672/
Test FAILed.

jkbradley · 2015-01-16T19:11:57Z

Also, as far as testing....it's hard. One way might be to:

Run RF with a random seed and subsampling rate 1.0
Run it the same way, but with with rate < 1.0
Make sure it learns different forests.
To make this test robust, you'll need to use the right (small but not too small) dataset size (maybe 5 features and 20 instances?), and also use a fixed random seed.

MechCoder · 2015-01-16T19:19:05Z

Thanks, Also a design decision, is it worthy enough to add this as an option to train given that it is now within the "style limit"?

jkbradley · 2015-01-16T19:23:44Z

I'd vote for not adding it to train since that part of the API is so unwieldy.

This reverts commit 6685b4494d2cb1ec72dbc540d2d747c75c6939ee.

SparkQA · 2015-01-17T15:12:42Z

Test build #25704 has started for PR 4073 at commit a7bfc70.

This patch merges cleanly.

SparkQA · 2015-01-17T16:21:27Z

Test build #25704 has finished for PR 4073 at commit a7bfc70.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-17T16:21:31Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25704/
Test PASSed.

MechCoder · 2015-01-17T17:27:12Z

@jkbradley I've added a test according to the other tests in the RandomForestSuite . Let me know if there is anything left.

SparkQA · 2015-01-17T17:27:41Z

Test build #25705 has started for PR 4073 at commit d1df1b2.

This patch merges cleanly.

SparkQA · 2015-01-17T18:36:37Z

Test build #25705 has finished for PR 4073 at commit d1df1b2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-17T18:36:40Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25705/
Test PASSed.

jkbradley · 2015-01-21T20:16:14Z

mllib/src/test/scala/org/apache/spark/mllib/tree/RandomForestSuite.scala

+  test("subsampling rate in RandomForest"){
+    val arr = EnsembleTestHelper.generateOrderedLabeledPoints(5, 20)
+    val rdd = sc.parallelize(arr)
+    val strategy1 = new Strategy(algo = Classification, impurity = Gini, maxDepth = 2,


I would make 1 instance of strategy, train rf1, modify the strategy's subsamplingRate, and train rf2. Simpler + more clearly using the same settings for other parameters

MechCoder · 2015-01-22T07:45:15Z

@jkbradley Thanks for the tip. Fixed. Anything more?

SparkQA · 2015-01-22T07:47:34Z

Test build #25953 has started for PR 4073 at commit 8a0acb5.

This patch merges cleanly.

SparkQA · 2015-01-22T07:48:29Z

Test build #25953 has finished for PR 4073 at commit 8a0acb5.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-22T07:48:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25953/
Test FAILed.

MechCoder · 2015-01-22T07:52:38Z

Repushed after fixing the style checks.

SparkQA · 2015-01-22T07:57:46Z

Test build #25955 has started for PR 4073 at commit d5d68e7.

This patch merges cleanly.

SparkQA · 2015-01-22T09:01:07Z

Test build #25955 has finished for PR 4073 at commit d5d68e7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-22T09:01:10Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25955/
Test PASSed.

SparkQA · 2015-01-22T09:02:43Z

Test build #25961 has started for PR 4073 at commit e0e0d9c.

This patch merges cleanly.

SparkQA · 2015-01-22T10:09:43Z

Test build #25961 has finished for PR 4073 at commit e0e0d9c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-22T10:09:47Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25961/
Test PASSed.

MechCoder · 2015-01-24T20:21:16Z

ping @jkbradley Could you please have a final look?

jkbradley · 2015-01-24T21:37:54Z

@MechCoder This is an addition instead of a correction, but I just realized that Strategy.assertValid() does not check subsamplingRate. Would you mind adding that check? The rest looks good to me. Thanks!

MechCoder · 2015-01-25T03:41:35Z

@jkbradley Fixed. I can haz merge?

SparkQA · 2015-01-25T03:42:42Z

Test build #26061 has started for PR 4073 at commit 8012fb2.

This patch merges cleanly.

SparkQA · 2015-01-25T04:52:25Z

Test build #26061 has finished for PR 4073 at commit 8012fb2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-01-25T04:52:29Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26061/
Test PASSed.

jkbradley · 2015-01-26T18:27:24Z

@MechCoder Thanks! LGTM

CC: @mengxr Note this is sort of an API change: RandomForest can now be run with subsampled rows. (But this seems fine to me since users could set subsamplingRate before---it just wouldn't do anything.)

MechCoder · 2015-01-27T03:30:23Z

@mengxr This can also be viewd as a bugfix which prevents overwriting of the param subSamplingRate, which was hardcoded to 1.0

mengxr · 2015-01-27T03:47:34Z

Merged into master. Thanks!

MechCoder closed this Jan 16, 2015

MechCoder deleted the spark-3726 branch January 16, 2015 18:58

MechCoder restored the spark-3726 branch January 16, 2015 19:03

MechCoder reopened this Jan 16, 2015

[SPARK-3726] Allow sampling_rate not equal to 1.0

a7bfc70

This reverts commit 6685b4494d2cb1ec72dbc540d2d747c75c6939ee.

MechCoder force-pushed the spark-3726 branch from 6685b44 to a7bfc70 Compare January 17, 2015 15:11

Add test to verify subsampling behavior

d1df1b2

MechCoder changed the title ~~[SPARK-3726] [MLlib] Allow sampling_rate not equal to 1.0~~ [SPARK-3726] [MLlib] Allow sampling_rate not equal to 1.0 in RandomForests Jan 18, 2015

jkbradley reviewed Jan 21, 2015
View reviewed changes

MechCoder force-pushed the spark-3726 branch from 8a0acb5 to d5d68e7 Compare January 22, 2015 07:52

TST: Add better test

e0e0d9c

MechCoder force-pushed the spark-3726 branch from d5d68e7 to e0e0d9c Compare January 22, 2015 08:57

Add test in Strategy

8012fb2

asfgit closed this in d6894b1 Jan 27, 2015

MechCoder deleted the spark-3726 branch January 27, 2015 06:57

[SPARK-3726] [MLlib] Allow sampling_rate not equal to 1.0 in RandomForests #4073

[SPARK-3726] [MLlib] Allow sampling_rate not equal to 1.0 in RandomForests #4073

Conversation

MechCoder commented Jan 16, 2015

SparkQA commented Jan 16, 2015

MechCoder commented Jan 16, 2015

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

MechCoder commented Jan 16, 2015

MechCoder commented Jan 16, 2015

jkbradley commented Jan 16, 2015

MechCoder commented Jan 16, 2015

SparkQA commented Jan 16, 2015

jkbradley commented Jan 16, 2015

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

jkbradley commented Jan 16, 2015

MechCoder commented Jan 16, 2015

jkbradley commented Jan 16, 2015

SparkQA commented Jan 17, 2015

SparkQA commented Jan 17, 2015

AmplabJenkins commented Jan 17, 2015

MechCoder commented Jan 17, 2015

SparkQA commented Jan 17, 2015

SparkQA commented Jan 17, 2015

AmplabJenkins commented Jan 17, 2015

jkbradley Jan 21, 2015

Choose a reason for hiding this comment

MechCoder commented Jan 22, 2015

SparkQA commented Jan 22, 2015

SparkQA commented Jan 22, 2015

AmplabJenkins commented Jan 22, 2015

MechCoder commented Jan 22, 2015

SparkQA commented Jan 22, 2015

SparkQA commented Jan 22, 2015

AmplabJenkins commented Jan 22, 2015

SparkQA commented Jan 22, 2015

SparkQA commented Jan 22, 2015

AmplabJenkins commented Jan 22, 2015

MechCoder commented Jan 24, 2015

jkbradley commented Jan 24, 2015

MechCoder commented Jan 25, 2015

SparkQA commented Jan 25, 2015

SparkQA commented Jan 25, 2015

AmplabJenkins commented Jan 25, 2015

jkbradley commented Jan 26, 2015

MechCoder commented Jan 27, 2015

mengxr commented Jan 27, 2015