Skip to content

[SPARK-19234][MLLib] AFTSurvivalRegression should fail fast when any labels are zero #16652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
admackin wants to merge 6 commits into from
Closed

[SPARK-19234][MLLib] AFTSurvivalRegression should fail fast when any labels are zero #16652

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f0e09c5
[SPARK-19234][MLLib] make sure label is positive for AFT regression
admackin Jan 20, 2017
0efbf0f
[SPARK-19234][MLLib] fix test suite to ensure no zero-labels get pass…
admackin Jan 20, 2017
cfdd286
[SPARK-19234][MLLib] added test case to ensure fast failure on zero l…
Jan 23, 2017
f645ab8
[SPARK-19234][MLLib] added a clue about test failure per code-review …
Jan 30, 2017
88aee2f
[SPARK-19234][MLLib] fixing indentation to match code guidelines
Jan 30, 2017
c855976
[SPARK-19234][MLLib] fixing imports to match scala style guidelines
Jan 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mllib/src/main/scala/org/apache/spark/ml/regression/AFTSurvivalRegression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,4 +644,5 @@ private class AFTCostFun(
*/
private[regression] case class AFTPoint(features: Vector, label: Double, censor: Double) {
require(censor == 1.0 || censor == 0.0, "censor of class AFTPoint must be 1.0 or 0.0")
require(label > 0.0, "label of AFTPoint must be positive")
}
17 changes: 14 additions & 3 deletions mllib/src/test/scala/org/apache/spark/ml/regression/AFTSurvivalRegressionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@
package org.apache.spark.ml.regression

import scala.util.Random

import org.apache.spark.SparkFunSuite
import org.apache.spark.{SparkException, SparkFunSuite}
Copy link
Member

@srowen srowen Jan 20, 2017

Choose a reason for hiding this comment

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

(Leave the blank)

Copy link
Member

@srowen srowen Jan 30, 2017

Choose a reason for hiding this comment

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

This is still causing a style check failure @admackin

import org.apache.spark.ml.{Estimator, Model}
import org.apache.spark.ml.linalg.{Vector, Vectors}
import org.apache.spark.ml.param.ParamsSuite
import org.apache.spark.ml.util.{DefaultReadWriteTest, MLTestingUtils}
import org.apache.spark.ml.util.TestingUtils._
import org.apache.spark.mllib.random.{ExponentialGenerator, WeibullGenerator}
import org.apache.spark.mllib.util.MLlibTestSparkContext
import org.apache.spark.sql.{DataFrame, Row}
import org.apache.spark.sql.{DataFrame, Row, SparkSession}

class AFTSurvivalRegressionSuite
extends SparkFunSuite with MLlibTestSparkContext with DefaultReadWriteTest {
Expand Down Expand Up @@ -400,6 +400,17 @@ class AFTSurvivalRegressionSuite
val trainer = new AFTSurvivalRegression()
trainer.fit(dataset)
}

test("SPARK-19234: Fail fast on zero-valued labels") {
val dataset = spark.createDataFrame(Seq(
(1.218, 1.0, Vectors.dense(1.560, -0.605)),
(0.000, 0.0, Vectors.dense(0.346, 2.158)), // ← generates error; zero labels invalid
(4.199, 0.0, Vectors.dense(0.795, -0.226)))).toDF("label", "censor", "features")
val aft = new AFTSurvivalRegression()
intercept[SparkException] {
Copy link
Contributor

@imatiach-msft imatiach-msft Jan 30, 2017

Choose a reason for hiding this comment

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

it's recommended to verify the error message using withClue, eg:
withClue("label of AFTPoint must be positive") {
intercept[SparkException] {
aft.fit(dataset)
}
}

aft.fit(dataset)
}
}
}

object AFTSurvivalRegressionSuite {
Expand Down
3 changes: 1 addition & 2 deletions mllib/src/test/scala/org/apache/spark/ml/util/MLTestingUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ object MLTestingUtils extends SparkFunSuite {
actuals.foreach(actual => check(expected, actual))

val dfWithStringLabels = spark.createDataFrame(Seq(
("0", 1, Vectors.dense(0, 2, 3), 0.0)
("1", 1, Vectors.dense(0, 2, 3), 0.0)
)).toDF("label", "weight", "features", "censor")
val thrown = intercept[IllegalArgumentException] {
estimator.fit(dfWithStringLabels)
Expand Down Expand Up @@ -156,7 +156,6 @@ object MLTestingUtils extends SparkFunSuite {
featuresColName: String = "features",
censorColName: String = "censor"): Map[NumericType, DataFrame] = {
val df = spark.createDataFrame(Seq(
(0, Vectors.dense(0)),
(1, Vectors.dense(1)),
(2, Vectors.dense(2)),
(3, Vectors.dense(3)),
Expand Down