[SPARK-8764][ML] string indexer should take option to handle unseen values #7266

holdenk · 2015-07-07T20:49:26Z

As a precursor to adding a public constructor add an option to handle unseen values by skipping rather than throwing an exception (default remains throwing an exception),

SparkQA · 2015-07-07T21:40:13Z

Test build #36715 has finished for PR 7266 at commit d69ef5e.

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

feynmanliang · 2015-07-08T22:06:44Z

mllib/src/test/scala/org/apache/spark/ml/feature/StringIndexerSuite.scala

remove newline

feynmanliang · 2015-07-08T22:29:45Z

mllib/src/main/scala/org/apache/spark/ml/feature/StringIndexer.scala

I don't think the two different exceptions add any new information (the user already knows if the model is configured to skip invalid or not). Also, this branch should never execute because of L152. Perhaps we can collapse this into a single exception?

SparkQA · 2015-07-08T22:54:23Z

Test build #36848 has finished for PR 7266 at commit 75ffa69.

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

…lid is true

SparkQA · 2015-07-09T20:52:58Z

Test build #36964 has finished for PR 7266 at commit aa5b093.

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

feynmanliang · 2015-07-09T21:00:25Z

LGTM

holdenk · 2015-07-13T17:58:01Z

cc @jkbradley for review since created the jira.

SparkQA · 2015-07-16T22:39:00Z

Test build #37544 has finished for PR 7266 at commit 100a39b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan

holdenk · 2015-08-01T08:21:01Z

jenkins, retest this please.

holdenk · 2015-08-01T08:23:45Z

@jkbradley if you have a chance to look at this PR too its in the same class/file as the last one.

SparkQA · 2015-08-01T08:59:47Z

Test build #186 has finished for PR 7266 at commit 100a39b.

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

SparkQA · 2015-08-01T09:03:16Z

Test build #39348 has finished for PR 7266 at commit 100a39b.

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

jkbradley · 2015-08-02T00:31:21Z

I was taking another look at it, and I like the setup. But one thing I had not thought of was a third option: creating a new label/index which all unseen values are mapped to. That would let users avoid filtering out rows, and instead replace the bad/unseen values with a default value. Rather than putting that in this PR, could you modify the Param to be a String, so that we can specify other options in the future?

jkbradley · 2015-08-02T00:35:09Z

mllib/src/test/scala/org/apache/spark/ml/feature/StringIndexerSuite.scala

SparkQA · 2015-08-06T00:42:09Z

Test build #39952 has finished for PR 7266 at commit 414e249.

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

SparkQA · 2015-08-06T01:31:12Z

Test build #39957 has finished for PR 7266 at commit 7f37f6e.

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

jkbradley · 2015-08-06T17:05:53Z

mllib/src/main/scala/org/apache/spark/ml/param/shared/SharedParamsCodeGen.scala

Please make docs more explicit. It's very helpful to users. Here, it'd be nice to state the options and what they mean: "Options: skip (filter out rows with bad values) or error (throw an error). Note that skipping rows means the Transformer can output a smaller dataset."

jkbradley · 2015-08-06T17:06:17Z

Those 2 small items are the only issues I see.

SparkQA · 2015-08-06T17:49:41Z

Test build #40054 has finished for PR 7266 at commit 045bf22.

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

SparkQA · 2015-08-06T18:07:38Z

Test build #40059 has finished for PR 7266 at commit 38a4de9.

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

holdenk · 2015-08-06T18:33:16Z

Catalyst failure seems likely unrelated, jenkins retest this please.

SparkQA · 2015-08-06T18:40:50Z

Test build #258 has finished for PR 7266 at commit 38a4de9.

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

SparkQA · 2015-08-06T19:22:23Z

Test build #40068 has finished for PR 7266 at commit 38a4de9.

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

jkbradley · 2015-08-11T18:32:40Z

LGTM, merging with master.
Thanks!

…values As a precursor to adding a public constructor add an option to handle unseen values by skipping rather than throwing an exception (default remains throwing an exception), Author: Holden Karau <[email protected]> Closes apache#7266 from holdenk/SPARK-8764-string-indexer-should-take-option-to-handle-unseen-values and squashes the following commits: 38a4de9 [Holden Karau] fix long line 045bf22 [Holden Karau] Add a second b entry so b gets 0 for sure 81dd312 [Holden Karau] Update the docs for handleInvalid param to be more descriptive 7f37f6e [Holden Karau] remove extra space (scala style) 414e249 [Holden Karau] And switch to using handleInvalid instead of skipInvalid 1e53f9b [Holden Karau] update the param (codegen side) 7a22215 [Holden Karau] fix typo 100a39b [Holden Karau] Merge in master aa5b093 [Holden Karau] Since we filter we should never go down this code path if getSkipInvalid is true 75ffa69 [Holden Karau] Remove extra newline d69ef5e [Holden Karau] Add a test b5734be [Holden Karau] Add support for unseen labels afecd4e [Holden Karau] Add a param to skip invalid entries.

miro-balaz · 2016-09-11T14:27:44Z

For me it would be usefull(when working with trees) if string indexer was mapping unseen labels to maximum known label+1. The row would not be skipped if it contained some non-used feature for classification.

holdenk · 2016-09-12T00:51:37Z

@miro-balaz : This probably isn't the best place for a new feature request - but if you head over to the ASF JIRA you can create a new ticket and cc the people who worked on this.

miro-balaz · 2016-09-12T04:53:24Z

thank you for directions

On Monday, 12 September 2016, Holden Karau [email protected] wrote:

@miro-balaz https://github.com/miro-balaz : This probably isn't the
best place for a new feature request - but if you head over to the ASF JIRA
you can create a new ticket and cc the people who worked on this.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7266 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALaD0wJm0im_ycl7fyxq-c97bY3RBCXkks5qpKJQgaJpZM4FT5k-
.

b-g-d · 2017-02-09T18:01:53Z

When "skip" is chosen as the way to handle Unseen labels, is there a way to know which rows were skipped?

holdenk added 3 commits July 7, 2015 11:47

Add a param to skip invalid entries.

afecd4e

Add support for unseen labels

b5734be

Add a test

d69ef5e

feynmanliang reviewed Jul 8, 2015
View reviewed changes

mllib/src/test/scala/org/apache/spark/ml/feature/StringIndexerSuite.scala Outdated

Copy link

Contributor

feynmanliang Jul 8, 2015

Choose a reason for hiding this comment

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

remove newline

Remove extra newline

75ffa69

feynmanliang reviewed Jul 8, 2015
View reviewed changes

Since we filter we should never go down this code path if getSkipInva…

aa5b093

…lid is true

Merge in master

100a39b

jkbradley reviewed Aug 2, 2015
View reviewed changes

mllib/src/test/scala/org/apache/spark/ml/feature/StringIndexerSuite.scala Outdated

Copy link

Member

jkbradley Aug 2, 2015

Choose a reason for hiding this comment

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

typo

holdenk added 3 commits August 5, 2015 17:16

fix typo

7a22215

update the param (codegen side)

1e53f9b

And switch to using handleInvalid instead of skipInvalid

414e249

remove extra space (scala style)

7f37f6e

jkbradley reviewed Aug 6, 2015
View reviewed changes

holdenk added 2 commits August 6, 2015 10:42

Update the docs for handleInvalid param to be more descriptive

81dd312

Add a second b entry so b gets 0 for sure

045bf22

fix long line

38a4de9

asfgit closed this in dbd778d Aug 11, 2015

[SPARK-8764][ML] string indexer should take option to handle unseen values #7266

[SPARK-8764][ML] string indexer should take option to handle unseen values #7266

Uh oh!

Conversation

holdenk commented Jul 7, 2015

Uh oh!

SparkQA commented Jul 7, 2015

Uh oh!

feynmanliang Jul 8, 2015

Choose a reason for hiding this comment

Uh oh!

feynmanliang Jul 8, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 8, 2015

Uh oh!

SparkQA commented Jul 9, 2015

Uh oh!

feynmanliang commented Jul 9, 2015

Uh oh!

holdenk commented Jul 13, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

holdenk commented Aug 1, 2015

Uh oh!

holdenk commented Aug 1, 2015

Uh oh!

SparkQA commented Aug 1, 2015

Uh oh!

SparkQA commented Aug 1, 2015

Uh oh!

jkbradley commented Aug 2, 2015

Uh oh!

jkbradley Aug 2, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

jkbradley Aug 6, 2015

Choose a reason for hiding this comment

Uh oh!

jkbradley commented Aug 6, 2015

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

holdenk commented Aug 6, 2015

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

SparkQA commented Aug 6, 2015

Uh oh!

jkbradley commented Aug 11, 2015

Uh oh!

miro-balaz commented Sep 11, 2016

Uh oh!

holdenk commented Sep 12, 2016

Uh oh!

miro-balaz commented Sep 12, 2016

Uh oh!

b-g-d commented Feb 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants