[GitHub] [spark] viirya commented on a change in pull request #28704: [SPARK-31777][ML] Add user-specified fold column to CrossValidator

2020-06-02 Thread GitBox


viirya commented on a change in pull request #28704:
URL: https://github.com/apache/spark/pull/28704#discussion_r434020686



##
File path: 
mllib/src/test/scala/org/apache/spark/ml/tuning/CrossValidatorSuite.scala
##
@@ -40,10 +41,22 @@ class CrossValidatorSuite
   import testImplicits._
 
   @transient var dataset: Dataset[_] = _
+  @transient var datasetWithFold: Dataset[_] = _
 
   override def beforeAll(): Unit = {
 super.beforeAll()
 dataset = sc.parallelize(generateLogisticInput(1.0, 1.0, 100, 42), 
2).toDF()
+val foldCol = udf { () =>
+  val r = Math.random()

Review comment:
   Good idea.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] viirya commented on a change in pull request #28704: [SPARK-31777][ML] Add user-specified fold column to CrossValidator

2020-06-02 Thread GitBox


viirya commented on a change in pull request #28704:
URL: https://github.com/apache/spark/pull/28704#discussion_r434017713



##
File path: mllib/src/main/scala/org/apache/spark/ml/tuning/CrossValidator.scala
##
@@ -56,6 +56,19 @@ private[ml] trait CrossValidatorParams extends 
ValidatorParams {
   def getNumFolds: Int = $(numFolds)
 
   setDefault(numFolds -> 3)
+
+  /**
+   * Param for the column name of user specified fold number. Once this is 
specified,
+   * `CrossValidator` won't do random k-fold split. Note that this column 
should be
+   * integer type with range [0, numFolds) and Spark won't do sanity-check for 
this

Review comment:
   For doing sanity check, seems we need a UDF doing the check/throwing 
error? Could be bad for performance? I think mod numFolds sounds a better one. 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org