sarutak commented on code in PR #59559:
URL: https://github.com/apache/spark/pull/59559#discussion_r4299368036
##########
mllib-local/src/test/scala/org/apache/spark/ml/linalg/MatricesSuite.scala:
##########
@@ -123,6 +123,30 @@ class MatricesSuite extends SparkMLFunSuite {
assert(!sparseMat.toArray.eq(sparseCopy.toArray))
}
+ test("DenseMatrix.hashCode is consistent with equals") {
+ val dm = new DenseMatrix(3, 2, Array(1.0, -0.0, 2.0, 0.0, 3.0, 0.0))
+ assert(dm.hashCode === dm.hashCode)
+ assert(dm.copy === dm)
+ assert(dm.copy.hashCode === dm.hashCode)
+
+ // The same matrix with the values stored in row major.
+ val dmt = new DenseMatrix(3, 2, Array(1.0, 0.0, -0.0, 3.0, 2.0, 0.0),
isTransposed = true)
+ val dmtColMajor = dmt.toDenseColMajor
+ assert(!dmtColMajor.isTransposed)
+ assert(dmt === dmtColMajor)
+ assert(dmt.hashCode === dmtColMajor.hashCode)
+
+ // The same matrix with an explicit 0.0 and an explicit -0.0.
+ val sm = new SparseMatrix(3, 2, Array(0, 3, 5), Array(0, 1, 2, 0, 1),
+ Array(1.0, 0.0, 2.0, -0.0, 3.0))
+ Seq(dm, dmt).foreach { m =>
+ assert(m === sm)
+ assert(m.hashCode === sm.hashCode)
+ assert(m.transpose === sm.transpose)
+ assert(m.transpose.hashCode === sm.transpose.hashCode)
Review Comment:
Non-blocking: the zeros used here are only `0.0` and `-0.0`, which are zero
both by value and by hash code, so the test does not pin "skip by hash code" in
`hashCode`. On cb903f5 without changing its non-test code, the test still
passes if `if (valueHash != 0)` is changed to `if (data(i) != 0.0)`. A nonzero
value can have a zero hash code, e.g. `4294967297.0` (`(1L << 32) + 1`, hashed
as a long, `1 ^ 1 = 0`), and Breeze skips it on the sparse side. How about
adding a case like this?
```scala
// A nonzero value whose hash code is 0 is skipped as well.
val x = 4294967297.0 // (1L << 32) + 1
assert(x.## === 0)
val dmx = new DenseMatrix(2, 1, Array(1.0, x))
assert(dmx === dmx.toSparse)
assert(dmx.hashCode === dmx.toSparse.hashCode)
```
I ran it locally: it passes on cb903f5 and fails with the change above
(`230572971 did not equal 652125558`). The mllib side delegates to
`asML.hashCode`, so the ml suite should be enough.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
