You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/03/17 10:33:30 UTC
[GitHub] [spark] zhengruifeng opened a new pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
zhengruifeng opened a new pull request #35893:
URL: https://github.com/apache/spark/pull/35893
### What changes were proposed in this pull request?
1, add validation methods in `DatasetUtils`;
2, use the new methods to check the input dataset;
### Why are the changes needed?
LinearSVC should fail fast if the input dataset contains invalid values:
```
import org.apache.spark.ml.feature._
import org.apache.spark.ml.linalg._
import org.apache.spark.ml.classification._
import org.apache.spark.ml.clustering._
val df = sc.parallelize(Seq(LabeledPoint(1.0, Vectors.dense(1.0, Double.NaN)), LabeledPoint(0.0, Vectors.dense(Double.PositiveInfinity, 2.0)))).toDF()
val svc = new LinearSVC()
val model = svc.fit(df)
scala> model.intercept
res0: Double = NaN
scala> model.coefficients
res1: org.apache.spark.ml.linalg.Vector = [NaN,NaN]
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
added test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1070770834
see https://issues.apache.org/jira/browse/SPARK-38584,
existing data validation should be unified, and this is the first PR to check LinearSVC.
friendly ping @srowen @huaxingao @WeichenXu123
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1071948976
just notice that UDF will skip null values, so directly using SQL functions is better since it can check NULL:
```
val df = sc.parallelize(Seq(("1", 2, 3.0), (null, 4, 5.0), ("0", 2, Double.PositiveInfinity))).toDF("a", "b", "c")
def checkBinaryLabel = udf {
label: Double =>
require(label == 0 || label == 1,
s"Labels MUST be in {0, 1}, but got $label")
label
}
scala> df.select(checkBinaryLabel(col("a").cast("double"))).show
+----------------------+
|UDF(cast(a as double))|
+----------------------+
| 1.0|
| null|
| 0.0|
+----------------------+
def checkBinaryLabels(labelCol: String): Column = {
val casted = col(labelCol).cast(DoubleType)
when(casted.isNull, raise_error(lit("Labels MUST NOT be NULL")))
.when(casted =!= 0 && casted =!= 1,
raise_error(concat(lit("Labels MUST be in {0, 1}, but got "), casted)))
.otherwise(casted)
}
scala> df.select(checkBinaryLabels("a")).show
22/03/18 09:32:59 ERROR Executor: Exception in task 2.0 in stage 5.0 (TID 19)
java.lang.RuntimeException: Labels MUST NOT be NULL
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #35893:
URL: https://github.com/apache/spark/pull/35893#discussion_r829082655
##########
File path: mllib/src/main/scala/org/apache/spark/ml/util/DatasetUtils.scala
##########
@@ -17,16 +17,57 @@
package org.apache.spark.ml.util
-import org.apache.spark.ml.linalg.{Vector, Vectors, VectorUDT}
+import org.apache.spark.ml.linalg._
import org.apache.spark.mllib.linalg.{Vector => OldVector, Vectors => OldVectors}
import org.apache.spark.rdd.RDD
import org.apache.spark.sql.{Column, Dataset, Row}
-import org.apache.spark.sql.functions.{col, udf}
+import org.apache.spark.sql.functions.{col, lit, udf}
import org.apache.spark.sql.types.{ArrayType, DoubleType, FloatType}
private[spark] object DatasetUtils {
+ private[ml] def getBinaryLabelCol(labelCol: String) = {
+ checkBinaryLabel(col(labelCol).cast(DoubleType))
+ }
+
+ private[ml] def getNonNegativeWeightCol(weightCol: Option[String]) = weightCol match {
+ case Some(w) if w.nonEmpty => checkNonNegativeWeight(col(w).cast(DoubleType))
+ case _ => lit(1.0)
+ }
+
+ private[ml] def getNonNanVectorCol(featuresCol: String) = {
+ checkNonNanVector(col(featuresCol))
+ }
+
+ private def checkBinaryLabel = udf {
Review comment:
Rather than use UDFs, I wonder if it's faster to just check whether isNaN() is true for any values using Spark SQL functions? likewise checking for 0/1, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1073891615
Sure, just logical groupings. If we're going to have 4-5 1-file PRs, then combine them. If some are extensive changes in themselves, OK, separate PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1071948976
just notice that UDF will skip null values, so directly using SQL functions is better since it can check NULL:
```
val df = sc.parallelize(Seq(("1", 2, 3.0), (null, 4, 5.0), ("0", 2, Double.PositiveInfinity))).toDF("a", "b", "c")
def checkBinaryLabel = udf {
label: Double =>
require(label == 0 || label == 1,
s"Labels MUST be in {0, 1}, but got $label")
label
}
scala> df.select(checkBinaryLabel(col("a").cast("double"))).show
+----------------------+
|UDF(cast(a as double))|
+----------------------+
| 1.0|
| null|
| 0.0|
+----------------------+
def checkBinaryLabels(labelCol: String): Column = {
val casted = col(labelCol).cast(DoubleType)
when(casted.isNull, raise_error(lit("Labels MUST NOT be NULL")))
.when(casted =!= 0 && casted =!= 1,
raise_error(concat(lit("Labels MUST be in {0, 1}, but got "), casted)))
.otherwise(casted)
}
scala> df.select(checkBinaryLabels("a")).show
22/03/18 09:32:59 ERROR Executor: Exception in task 2.0 in stage 5.0 (TID 19)
java.lang.RuntimeException: Labels MUST NOT be NULL
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1073390561
@srowen OK. I will have a try.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #35893:
URL: https://github.com/apache/spark/pull/35893#discussion_r829082655
##########
File path: mllib/src/main/scala/org/apache/spark/ml/util/DatasetUtils.scala
##########
@@ -17,16 +17,57 @@
package org.apache.spark.ml.util
-import org.apache.spark.ml.linalg.{Vector, Vectors, VectorUDT}
+import org.apache.spark.ml.linalg._
import org.apache.spark.mllib.linalg.{Vector => OldVector, Vectors => OldVectors}
import org.apache.spark.rdd.RDD
import org.apache.spark.sql.{Column, Dataset, Row}
-import org.apache.spark.sql.functions.{col, udf}
+import org.apache.spark.sql.functions.{col, lit, udf}
import org.apache.spark.sql.types.{ArrayType, DoubleType, FloatType}
private[spark] object DatasetUtils {
+ private[ml] def getBinaryLabelCol(labelCol: String) = {
+ checkBinaryLabel(col(labelCol).cast(DoubleType))
+ }
+
+ private[ml] def getNonNegativeWeightCol(weightCol: Option[String]) = weightCol match {
+ case Some(w) if w.nonEmpty => checkNonNegativeWeight(col(w).cast(DoubleType))
+ case _ => lit(1.0)
+ }
+
+ private[ml] def getNonNanVectorCol(featuresCol: String) = {
+ checkNonNanVector(col(featuresCol))
+ }
+
+ private def checkBinaryLabel = udf {
Review comment:
Rather than use UDFs, I wonder if it's faster to just check whether isNaN() is true for any values using Spark SQL functions? likewise checking for 0/1, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1076968039
> What would happen, just silently get bad results?
yes, like the case in the description:
https://github.com/apache/spark/pull/35893#issue-1172201935
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1073188159
@srowen
hmm, I plan to make other 4~5 PR for other impls: other classification/regression/clustering/recommend/etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a change in pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on a change in pull request #35893:
URL: https://github.com/apache/spark/pull/35893#discussion_r829616989
##########
File path: mllib/src/main/scala/org/apache/spark/ml/util/DatasetUtils.scala
##########
@@ -17,16 +17,57 @@
package org.apache.spark.ml.util
-import org.apache.spark.ml.linalg.{Vector, Vectors, VectorUDT}
+import org.apache.spark.ml.linalg._
import org.apache.spark.mllib.linalg.{Vector => OldVector, Vectors => OldVectors}
import org.apache.spark.rdd.RDD
import org.apache.spark.sql.{Column, Dataset, Row}
-import org.apache.spark.sql.functions.{col, udf}
+import org.apache.spark.sql.functions.{col, lit, udf}
import org.apache.spark.sql.types.{ArrayType, DoubleType, FloatType}
private[spark] object DatasetUtils {
+ private[ml] def getBinaryLabelCol(labelCol: String) = {
+ checkBinaryLabel(col(labelCol).cast(DoubleType))
+ }
+
+ private[ml] def getNonNegativeWeightCol(weightCol: Option[String]) = weightCol match {
+ case Some(w) if w.nonEmpty => checkNonNegativeWeight(col(w).cast(DoubleType))
+ case _ => lit(1.0)
+ }
+
+ private[ml] def getNonNanVectorCol(featuresCol: String) = {
+ checkNonNanVector(col(featuresCol))
+ }
+
+ private def checkBinaryLabel = udf {
Review comment:
good idea, will have a try
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
zhengruifeng closed pull request #35893:
URL: https://github.com/apache/spark/pull/35893
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a change in pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on a change in pull request #35893:
URL: https://github.com/apache/spark/pull/35893#discussion_r829616989
##########
File path: mllib/src/main/scala/org/apache/spark/ml/util/DatasetUtils.scala
##########
@@ -17,16 +17,57 @@
package org.apache.spark.ml.util
-import org.apache.spark.ml.linalg.{Vector, Vectors, VectorUDT}
+import org.apache.spark.ml.linalg._
import org.apache.spark.mllib.linalg.{Vector => OldVector, Vectors => OldVectors}
import org.apache.spark.rdd.RDD
import org.apache.spark.sql.{Column, Dataset, Row}
-import org.apache.spark.sql.functions.{col, udf}
+import org.apache.spark.sql.functions.{col, lit, udf}
import org.apache.spark.sql.types.{ArrayType, DoubleType, FloatType}
private[spark] object DatasetUtils {
+ private[ml] def getBinaryLabelCol(labelCol: String) = {
+ checkBinaryLabel(col(labelCol).cast(DoubleType))
+ }
+
+ private[ml] def getNonNegativeWeightCol(weightCol: Option[String]) = weightCol match {
+ case Some(w) if w.nonEmpty => checkNonNegativeWeight(col(w).cast(DoubleType))
+ case _ => lit(1.0)
+ }
+
+ private[ml] def getNonNanVectorCol(featuresCol: String) = {
+ checkNonNanVector(col(featuresCol))
+ }
+
+ private def checkBinaryLabel = udf {
Review comment:
good idea, will have a try
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of LinearSVC
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1073252693
Is it possible to include those changes here? Those sound small and related. Rather than 4 or 5 of them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1076965952
What would happen, just silently get bad results?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1076964036
> Looks OK. As far as you know, would this change behavior in any case? like something results in an error now that did not before?
yes. at least for LinearSVC, it will not throw an error before. I think we may need to notice this change in migration doc for 3.4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1077338419
Merged to master, thanks all!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #35893: [SPARK-38588][ML] Validate input dataset of ml.classification
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on pull request #35893:
URL: https://github.com/apache/spark/pull/35893#issuecomment-1073848464
@srowen I found that it will touch too many files and some changes like to be impl-specific, what about limiting the scope of this PR to classification only?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org