Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-209044607
@mengxr Sure I will create a pull request shortly. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-209008233
@yongtang Sorry that I just found this was merged. I think it might be
better to use `Int.parseInt` and `parseDouble` instead of regexes, which is
less robust. For
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11989
---
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
Github user MLnick commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208947738
LGTM. Merged to master. Thanks @yongtang and also @sethah for reviewing!
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208928721
**[Test build #55606 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55606/consoleFull)**
for PR 11989 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208929077
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208929081
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208911357
**[Test build #55606 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55606/consoleFull)**
for PR 11989 at commit
Github user MLnick commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-208910425
jenkins retest this please
---
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-207478267
Merged build finished. Test FAILed.
---
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-207478269
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-207477509
Hi @MLnick I updated the pull request to add additional test cases to cover
invalid values. Let me know if there is any other issues. Thanks.
---
If your project is
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206931189
Hi @MLnick, here is the complete list of the scenarios:
Real numbers (has to contain `.`), assume number of features is 15
```
.0not allowed
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58832650
--- Diff:
mllib/src/test/java/org/apache/spark/ml/regression/JavaRandomForestRegressorSuite.java
---
@@ -80,6 +80,14 @@ public void runDT() {
for
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58832506
--- Diff:
mllib/src/test/java/org/apache/spark/ml/regression/JavaRandomForestRegressorSuite.java
---
@@ -80,6 +80,14 @@ public void runDT() {
for
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206445870
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206445872
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206445727
**[Test build #55122 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55122/consoleFull)**
for PR 11989 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206442210
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206442214
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206442042
**[Test build #55121 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55121/consoleFull)**
for PR 11989 at commit
Github user yongtang commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58725975
--- Diff:
mllib/src/test/scala/org/apache/spark/ml/tree/impl/RandomForestSuite.scala ---
@@ -422,6 +422,13 @@ class RandomForestSuite extends SparkFunSuite
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206427746
**[Test build #55121 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55121/consoleFull)**
for PR 11989 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206431872
**[Test build #55122 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55122/consoleFull)**
for PR 11989 at commit
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58657961
--- Diff:
mllib/src/test/scala/org/apache/spark/ml/tree/impl/RandomForestSuite.scala ---
@@ -422,6 +422,13 @@ class RandomForestSuite extends SparkFunSuite
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206087350
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206087349
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206087258
**[Test build #55071 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55071/consoleFull)**
for PR 11989 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206071092
**[Test build #55071 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55071/consoleFull)**
for PR 11989 at commit
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-206070686
Thanks @MLnick. The pull request has been updated. Please let me know if
there are other issues.
---
If your project is set up for it, you can reply to this email
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58593340
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/tree/RandomForest.scala ---
@@ -55,10 +55,16 @@ import org.apache.spark.util.Utils
* @param
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58592767
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -329,6 +329,8 @@ private[ml] trait HasFeatureSubsetStrategy extends
Params {
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205893715
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205893720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205893381
**[Test build #54995 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54995/consoleFull)**
for PR 11989 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205878822
**[Test build #54995 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54995/consoleFull)**
for PR 11989 at commit
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205877757
@sethah Thanks. The import has been removed.
---
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
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205857272
This LGTM other than one small comment about imports. @MLnick could you
make a final pass?
---
If your project is set up for it, you can reply to this email and have
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58558671
--- Diff:
mllib/src/test/scala/org/apache/spark/ml/tree/impl/RandomForestSuite.scala ---
@@ -27,6 +27,7 @@ import
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205637767
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205637765
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205637544
**[Test build #54947 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54947/consoleFull)**
for PR 11989 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205628259
**[Test build #54947 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54947/consoleFull)**
for PR 11989 at commit
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-205627838
@sethah @MLnick Thanks so much for detailed review. The pull request has
been updated with the issues addressed. Let me know if there are any other
issues.
---
If
Github user yongtang commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58481526
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,8 @@ private[ml] trait RandomForestParams extends
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58481169
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,8 @@ private[ml] trait RandomForestParams extends
Github user yongtang commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58478120
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,8 @@ private[ml] trait RandomForestParams extends
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58402045
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -343,6 +343,8 @@ private[ml] trait RandomForestParams extends
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58402132
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/tree/RandomForest.scala ---
@@ -55,10 +55,15 @@ import org.apache.spark.util.Utils
* @param
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58400357
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,8 @@ private[ml] trait RandomForestParams extends
Github user MLnick commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58340397
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,8 @@ private[ml] trait RandomForestParams extends
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204641056
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204641058
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204641027
**[Test build #54754 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54754/consoleFull)**
for PR 11989 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204637508
**[Test build #54754 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54754/consoleFull)**
for PR 11989 at commit
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204637339
@sethah Thanks for the review. I just updated the pull request with issues
addressed. Let me know if there are any further issues.
---
If your project is set up for
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204637070
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204637047
**[Test build #54750 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54750/consoleFull)**
for PR 11989 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204637069
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204632243
**[Test build #54750 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54750/consoleFull)**
for PR 11989 at commit
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58240655
--- Diff:
mllib/src/test/java/org/apache/spark/ml/classification/JavaRandomForestClassifierSuite.java
---
@@ -80,6 +80,12 @@ public void runDT() {
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58239918
--- Diff:
mllib/src/test/scala/org/apache/spark/ml/tree/impl/RandomForestSuite.scala ---
@@ -509,6 +510,25 @@ class RandomForestSuite extends SparkFunSuite
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r58237319
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/tree/impl/DecisionTreeMetadata.scala
---
@@ -183,11 +183,16 @@ private[spark] object
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204306695
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204306558
**[Test build #54692 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54692/consoleFull)**
for PR 11989 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204306689
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204297156
**[Test build #54692 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54692/consoleFull)**
for PR 11989 at commit
Github user MLnick commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204296175
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
Github user sethah commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204225070
@yongtang I believe only Spark committers can do that. Maybe @jkbradley
could help?
---
If your project is set up for it, you can reply to this email and have your
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204204858
Hi @sethah by the way, could you help start a Jenkins test if possible?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user yongtang commented on the pull request:
https://github.com/apache/spark/pull/11989#issuecomment-204204359
Hi @sethah thanks for the review. There are some issues with the regex but
managed to get it done. The test also has been moved to ML. Let me know if
there are any
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r57978664
--- Diff:
mllib/src/test/scala/org/apache/spark/mllib/tree/RandomForestSuite.scala ---
@@ -135,6 +135,23 @@ class RandomForestSuite extends SparkFunSuite
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r57978546
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/tree/impl/DecisionTreeMetadata.scala
---
@@ -183,11 +183,40 @@ private[spark] object
Github user sethah commented on a diff in the pull request:
https://github.com/apache/spark/pull/11989#discussion_r57977742
--- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala ---
@@ -360,7 +362,9 @@ private[ml] trait RandomForestParams extends
74 matches
Mail list logo
