[SPARK-17207][MLLIB]fix comparing Vector bug in TestingUtils#14785
[SPARK-17207][MLLIB]fix comparing Vector bug in TestingUtils#14785mpjlu wants to merge 6 commits intoapache:masterfrom
Conversation
| def absTol(eps: Double): CompareVectorRightSide = CompareVectorRightSide( | ||
| (x: Vector, y: Vector, eps: Double) => { | ||
| x.toArray.zip(y.toArray).forall(x => x._1 ~= x._2 absTol eps) | ||
| if (x.size != y.size) { |
There was a problem hiding this comment.
How about just x.size == y.size && ...?
I think this also needs to be done for the matrix ops below.
There was a problem hiding this comment.
Yes, thanks.
How about check matrix length by
x.numRows == y.numRows && x.numCols == y.numCols && ... ?
|
Jenkins test this please |
|
Jenkins add to whitelist |
|
Test build #64349 has finished for PR 14785 at commit
|
|
Can you also fix https://github.com/apache/spark/blob/master/mllib-local/src/test/scala/org/apache/spark/ml/util/TestingUtils.scala? Please add tests showing the issue is addressed. Thanks. |
|
Sure, I will fix it, and add test cases. thanks. @dbtsai , |
|
Hi @dbtsai , PR 2294 added Matrix comparing in TestingUtils, but did not add any test cases in TestingUtilsSuite. I did not add test cases for Matrix comparing in the PR either. |
|
Test build #64418 has finished for PR 14785 at commit
|
|
Test build #64424 has finished for PR 14785 at commit
|
|
Please also add test cases for matrices. Thanks. |
|
Test build #64462 has finished for PR 14785 at commit
|
|
LGTM. Merged into master. Thanks. |
4 participants
What changes were proposed in this pull request?
fix comparing Vector bug in TestingUtils.
There is the same bug for Matrix comparing. How to check the length of Matrix should be discussed first.
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)