[SPARK-15141][EXAMPLE][DOC] Update OneVsRest Examples #12920

zhengruifeng · 2016-05-05T05:04:35Z

What changes were proposed in this pull request?

1, Add python example for OneVsRest
2, remove args-parsing

How was this patch tested?

manual tests
./bin/spark-submit examples/src/main/python/ml/one_vs_rest_example.py

zhengruifeng · 2016-05-05T05:06:57Z

There seems no similar API to directly get numClass like scala
val numClasses = MetadataUtils.getNumClasses(predictionColSchema).get

SparkQA · 2016-05-05T05:08:16Z

Test build #57841 has finished for PR 12920 at commit 474e252.

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

SparkQA · 2016-05-05T05:20:29Z

Test build #57842 has finished for PR 12920 at commit 44875ed.

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

SparkQA · 2016-05-05T06:01:40Z

Test build #57843 has finished for PR 12920 at commit 63d90ed.

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

HyukjinKwon · 2016-05-05T08:50:37Z

I am not sure if fixing examples can have the component [DOC] in the title. I saw [EXAMPLE] component was used by @dongjoon-hyun. This is a pretty minor but making good examples of PRs will help all other contributors.

@dongjoon-hyun Do you mind If I ask your thoughts please?

MLnick · 2016-05-05T13:01:15Z

@HyukjinKwon I get your concern, but these examples are actually directly included in the HTML documentation for Spark, via include_example, so they are both examples and docs. For this kind of thing that touches the HTML doc I think [DOC] is important to include.

MLnick · 2016-05-05T13:04:33Z

@zhengruifeng I think the OneVsRest examples are a little bloated. Could we turn them into simpler versions (essentially just the run part of the example) similar to say GradientBoostedTreeClassifierExample. Same applies for Java.

Frankly, the way the example looks now in HTML doc is very confusing with the reference to params that makes the self-contained example harder to follow.

zhengruifeng · 2016-05-05T13:25:17Z

@MLnick Agreed. I will remove the args-parsing blocks in the three example files.

yanboliang · 2016-05-05T13:46:27Z

+1 @MLnick

There are two types of example code under examples/:

Example applications (the ones with command-line parsing). Those are served as template code for users to build their own standalone applications. We should create those only for important algorithms.
Tutorial code. Those are for the user guide, usually without options to play with. We should keep one example per algorithm.

yanboliang · 2016-05-05T14:07:14Z

examples/src/main/python/ml/one_vs_rest_example.py

Use SparkSession instead of SQLContext. See #12809 for details.

SparkQA · 2016-05-06T02:32:58Z

Test build #57948 has finished for PR 12920 at commit a8d2681.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-05-06T03:10:55Z

Test build #57952 has finished for PR 12920 at commit 06cab7f.

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

zhengruifeng · 2016-05-06T04:40:59Z

@MLnick Args-Parsing was removed in those examples

MLnick · 2016-05-07T09:29:51Z

examples/src/main/scala/org/apache/spark/examples/ml/OneVsRestExample.scala

I think we should prefer to use ml.evaluation.MulticlassClassificationEvaluator here as it's a DataFrame API example. You may have to change the metric used - see DecisionTreeClassificationExample for example

MLnick · 2016-05-07T09:38:17Z

@zhengruifeng made a few comments - most importantly it's better to use the ml evaluators throughout as these are DataFrame API examples.

zhengruifeng · 2016-05-08T02:20:16Z

@MLnick

I have updated codes according to your comments.
Because MulticlassClassificationEvaluator do not support confusionMatrix, I just remove the computaion of confusionMatrix, and use Test Error instead.

SparkQA · 2016-05-08T02:45:20Z

Test build #58082 has finished for PR 12920 at commit af95019.

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

MLnick · 2016-05-08T05:32:20Z

Just use accuracy I think similar to the decision tree example

On Sun, 8 May 2016 at 04:21, Ruifeng Zheng notifications@github.com wrote:

@MLnick https://github.com/MLnick MulticlassClassificationEvaluator do
not support confusionMatrix not. So I will just remove the computaion of
confusionMatrix.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#12920 (comment)

srowen · 2016-05-08T08:09:07Z

examples/src/main/java/org/apache/spark/examples/ml/JavaOneVsRestExample.java

Nit: println

SparkQA · 2016-05-08T10:00:28Z

Test build #58100 has finished for PR 12920 at commit 16c2e74.

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

MLnick · 2016-05-10T19:59:11Z

examples/src/main/java/org/apache/spark/examples/ml/JavaOneVsRestExample.java

This import is no longer required I believe

MLnick · 2016-05-10T20:06:10Z

@zhengruifeng made a few comments. Pending those I think this is ready

zhengruifeng · 2016-05-11T01:38:48Z

@MLnick Thanks. Updated

SparkQA · 2016-05-11T01:51:43Z

Test build #58306 has finished for PR 12920 at commit 77ff733.

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

MLnick · 2016-05-11T07:09:59Z

examples/src/main/java/org/apache/spark/examples/ml/JavaOneVsRestExample.java

+    OneVsRest ovr = new OneVsRest().setClassifier(classifier);

-    // train the multiclass model
+    // train the multiclass model.


Sorry one last thing - here we use train.cache() but we don't do that in the other examples. Actually in general we don't seem to do that in any other examples from a quick look. So perhaps remove that and just do ovr.fit(train);

Right. I will fix it

SparkQA · 2016-05-11T07:36:48Z

Test build #58344 has finished for PR 12920 at commit 9049002.

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

MLnick · 2016-05-11T07:54:17Z

LGTM, merged to master and branch-2.0. Thanks!

## What changes were proposed in this pull request? 1, Add python example for OneVsRest 2, remove args-parsing ## How was this patch tested? manual tests `./bin/spark-submit examples/src/main/python/ml/one_vs_rest_example.py` Author: Zheng RuiFeng <ruifengz@foxmail.com> Closes #12920 from zhengruifeng/ovr_pe. (cherry picked from commit ad1a846) Signed-off-by: Nick Pentreath <nickp@za.ibm.com>

zhengruifeng changed the title ~~[SPARK-15141][DOC] Add python example for OneVsRest~~ [SPARK-15141][EXAMPLE] Add python example for OneVsRest May 5, 2016

yanboliang reviewed May 5, 2016
View reviewed changes

zhengruifeng changed the title ~~[SPARK-15141][EXAMPLE] Add python example for OneVsRest~~ [SPARK-15141][EXAMPLE][DOC] Add python example for OneVsRest May 6, 2016

zhengruifeng force-pushed the ovr_pe branch from 63d90ed to a8d2681 Compare May 6, 2016 02:22

MLnick reviewed May 7, 2016
View reviewed changes

zhengruifeng force-pushed the ovr_pe branch from 06cab7f to af95019 Compare May 8, 2016 02:35

zhengruifeng changed the title ~~[SPARK-15141][EXAMPLE][DOC] Add python example for OneVsRest~~ [SPARK-15141][EXAMPLE][DOC] Update OneVsRest Examples May 8, 2016

srowen reviewed May 8, 2016
View reviewed changes

MLnick reviewed May 10, 2016
View reviewed changes

examples/src/main/java/org/apache/spark/examples/ml/JavaOneVsRestExample.java Outdated

Copy link

Contributor

MLnick May 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is no longer required I believe

zhengruifeng added 11 commits May 11, 2016 09:32

create

56478be

finish pr

8bcc0c4

use argparse

3d67173

fix py style

4e08c90

add link in user guide doc

b80b1ec

update OvR examples, del the args-parsing

985a060

add implicits for dataset.as

ff84ccc

fix one nit

aec9994

use MulticlassClassificationEvaluator

78f503d

java print -> println

4538b64

del one unused import and update comments

77ff733

zhengruifeng force-pushed the ovr_pe branch from 16c2e74 to 77ff733 Compare May 11, 2016 01:37

MLnick reviewed May 11, 2016
View reviewed changes

rename cache in Java

9049002

asfgit closed this in ad1a846 May 11, 2016

zhengruifeng deleted the ovr_pe branch May 11, 2016 08:08

[SPARK-15141][EXAMPLE][DOC] Update OneVsRest Examples #12920

[SPARK-15141][EXAMPLE][DOC] Update OneVsRest Examples #12920

Uh oh!

Conversation

zhengruifeng commented May 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

zhengruifeng commented May 5, 2016

Uh oh!

SparkQA commented May 5, 2016

Uh oh!

SparkQA commented May 5, 2016

Uh oh!

SparkQA commented May 5, 2016

Uh oh!

HyukjinKwon commented May 5, 2016

Uh oh!

MLnick commented May 5, 2016

Uh oh!

MLnick commented May 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhengruifeng commented May 5, 2016

Uh oh!

yanboliang commented May 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 6, 2016

Uh oh!

SparkQA commented May 6, 2016

Uh oh!

zhengruifeng commented May 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MLnick commented May 7, 2016

Uh oh!

zhengruifeng commented May 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented May 8, 2016

Uh oh!

MLnick commented May 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MLnick commented May 10, 2016

Uh oh!

zhengruifeng commented May 11, 2016

Uh oh!

SparkQA commented May 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 11, 2016

Uh oh!

MLnick commented May 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zhengruifeng commented May 5, 2016 •

edited

Loading

MLnick commented May 5, 2016 •

edited

Loading

yanboliang commented May 5, 2016 •

edited

Loading

zhengruifeng commented May 8, 2016 •

edited

Loading