[SparkR][SPARK-21381]:SparkR: pass on setHandleInvalid for classification algorithms

[wangmiao1981]

What changes were proposed in this pull request?

SPARK-20307 Added handleInvalid option to RFormula for tree-based classification algorithms. We should add this parameter for other classification algorithms in SparkR.

This is a followup PR for SPARK-20307.

How was this patch tested?

New Unit tests are added.

[SparkQA]

Test build #79546 has finished for PR 18605 at commit 77b04a3.

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

[wangmiao1981]

@felixcheung This is a follow-up PR of JIRA-20307.

[wangmiao1981]

Trigger windows check.

[wangmiao1981]

Reopen for windows check

[felixcheung]

could you update this to make it consistent with the earlier PR? I think it's mostly the param document wording

[wangmiao1981]

Sure. I am reading the #18613 comments. Just come back from a business travel. Thanks!

[SparkQA]

Test build #79678 has finished for PR 18605 at commit e59941e.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[wangmiao1981]

@yanboliang after #18613, unit tests fails if "skip" is used.

For example,
data <- data.frame(clicked = base::sample(c(0, 1), 10, replace = TRUE),
someString = base::sample(c("this", "that"), 10, replace = TRUE),
stringsAsFactors = FALSE)
trainidxs <- base::sample(nrow(data), nrow(data) * 0.7)
traindf <- as.DataFrame(data[trainidxs, ])
testdf <- as.DataFrame(rbind(data[-trainidxs, ], c(0, "the other")))
model <- spark.mlp(traindf, clicked ~ ., layers = c(1, 3), handleInvalid = "skip")
predictions <- predict(model, testdf)
expect_equal(class(collect(predictions)$clicked[1]), "character")

It fails the as if "error" is used.

If I change "skip" to "keep", then the predictions$click[0] is NULL.

collect(predictions)
[1] clicked someString prediction
<0 rows> (or 0-length row.names)
collect(predictions)$click[1]
[[1]]
NULL

I am not sure whether this is expected or there is a bug.

Before, the units work fine.

[yanboliang]

@wangmiao1981 This is expected, see my comment here . This uncovers an existing bug for forceIndexLabel. I will send a fix later, please use keep to test handleInvalid currently. Thanks.

[wangmiao1981]

@yanboliang Thanks for your reply! I will change the unit tests now.

[SparkQA]

Test build #79728 has finished for PR 18605 at commit 3d7c517.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #79734 has finished for PR 18605 at commit 3ebb5cd.

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

[wangmiao1981]

@yanboliang I have made changes accordingly. Thanks!

[wangmiao1981]

@felixcheung Can you take a look? Thanks!

[felixcheung]

I'll take a look

[felixcheung]

sorry about the delay, one comment otherwise LG

[felixcheung]

this was only for classification though, so the original text in classification model. we should keep.
ditto for decisionTree and gbt in this .R file

[SparkQA]

Test build #80081 has finished for PR 18605 at commit 18c69d2.

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

[felixcheung]

merged to master