[SPARK-22884][ML][TESTS] ML test for StructuredStreaming: spark.ml.clustering #20319
Conversation
| import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder | ||
|
|
||
| private[clustering] object Encoders { | ||
| implicit val vectorEncoder = ExpressionEncoder[Vector]() |
There was a problem hiding this comment.
Is there a better solution to provide an implicit Encoder[Vector] for testTransformer?
Is it ok here, or is there a better place for it?
e.g. org.apache.spark.mllib.util.MLlibTestSparkContext.testImplicits
There was a problem hiding this comment.
Thanks for asking; you shouldn't need to do this. I'll comment on BisectingKMeansSuite.scala
about using testImplicits instead. You basically just need to import testImplicits._ and use Tuple1 for the type param for testTransformer.
|
Jenkins, add to whitelist |
|
Test build #86391 has finished for PR 20319 at commit
|
|
@jkbradley could you check out this change, please? |
|
Test build #86479 has finished for PR 20319 at commit
|
|
@smurakozi Thanks for the PR! I have bandwidth to review this now. Do you have time to rebase this to fix the merge conflicts? |
|
@smurakozi Thanks for the PR! Could you resolve conflicts first? and then I will make a review. If you're busy I can also take over it. |
|
Test build #89063 has finished for PR 20319 at commit
|
|
@jkbradley, @WeichenXu123 thanks for checking it out. I've resolved the conflicts, build is green. |
|
Reviewing now! |
| import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder | ||
|
|
||
| private[clustering] object Encoders { | ||
| implicit val vectorEncoder = ExpressionEncoder[Vector]() |
There was a problem hiding this comment.
Thanks for asking; you shouldn't need to do this. I'll comment on BisectingKMeansSuite.scala
about using testImplicits instead. You basically just need to import testImplicits._ and use Tuple1 for the type param for testTransformer.
| extends SparkFunSuite with MLlibTestSparkContext with DefaultReadWriteTest { | ||
| class BisectingKMeansSuite extends MLTest with DefaultReadWriteTest { | ||
|
|
||
| import Encoders._ |
There was a problem hiding this comment.
import testImplicits._ instead
| // Verify we hit the edge case | ||
| assert(numClusters < k && numClusters > 1) | ||
|
|
||
| testTransformerByGlobalCheckFunc[Vector](sparseDataset.toDF(), model, "prediction") { rows => |
There was a problem hiding this comment.
Use Tuple1[Vector] instead of Vector
| val clusters = rows.map(_.getAs[Int](predictionColName)).toSet | ||
| assert(clusters.size === k) | ||
| assert(clusters === Set(0, 1, 2, 3, 4)) | ||
| assert(model.computeCost(dataset) < 0.1) |
There was a problem hiding this comment.
These checks which do not use "rows" should go outside of testTransformerByGlobalCheckFunc
|
@smurakozi Do you have time to update this? I did a full review, though it now has a small merge conflict. Thanks! |
|
I'm going to take this over to get this done, but @smurakozi you'll be the primary author. I'll link the PR here in a minute |
|
Done! Here it is: #21358 @smurakozi Could you please close this issue and help review the new PR if you have time? Thanks! |
## What changes were proposed in this pull request? Converting clustering tests to also check code with structured streaming, using the ML testing infrastructure implemented in SPARK-22882. This PR is a new version of #20319 Author: Sandor Murakozi <[email protected]> Author: Joseph K. Bradley <[email protected]> Closes #21358 from jkbradley/smurakozi-SPARK-22884.
|
Can one of the admins verify this patch? |
6 participants
What changes were proposed in this pull request?
Converting clustering tests to also check code with structured streaming, using the ML testing infrastructure implemented in SPARK-22882.
How was this patch tested?
N/A