From 6416fa0b5badf46293f90410291fe0d41f4f704f Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Fri, 7 Aug 2015 15:00:17 -0700 Subject: [PATCH 1/6] Add failing sparse matrix equals tests --- .../spark/mllib/linalg/MatricesSuite.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala index a270ba2562db9..57fbb78c4b85c 100644 --- a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala +++ b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala @@ -74,6 +74,22 @@ class MatricesSuite extends SparkFunSuite { } } + test("equals") { + val dm1 = Matrices.dense(2, 2, Array(0.0, 1.0, 2.0, 3.0)) + assert(dm1 === dm1) + assert(dm1 !== dm1.transpose) + + val dm2 = Matrices.dense(2, 2, Array(0.0, 2.0, 1.0, 3.0)) + assert(dm1 === dm2.transpose) + + val sm1 = dm1.toSparse + assert(sm1 === sm1) + assert(sm1 !== sm1.transpose) + + val sm2 = dm2.toSparse + assert(sm1 === sm2.transpose) + } + test("matrix copies are deep copies") { val m = 3 val n = 2 From 43d28fa2b9e7c2705b26df821bb07ca807f78e59 Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Fri, 7 Aug 2015 15:04:40 -0700 Subject: [PATCH 2/6] Fix failing test --- .../main/scala/org/apache/spark/mllib/linalg/Matrices.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala index 1c858348bf20e..bae90c787a4c1 100644 --- a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala +++ b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala @@ -519,6 +519,11 @@ class SparseMatrix( rowIndices: Array[Int], values: Array[Double]) = this(numRows, numCols, colPtrs, rowIndices, values, false) + override def equals(o: Any): Boolean = o match { + case m: SparseMatrix => toBreeze == m.toBreeze + case _ => false + } + private[mllib] def toBreeze: BM[Double] = { if (!isTransposed) { new BSM[Double](values, numRows, numCols, colPtrs, rowIndices) From 78f942662cf01a60d52b3fa6fb7b9e5aaae194d2 Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Fri, 7 Aug 2015 15:09:43 -0700 Subject: [PATCH 3/6] Add casts --- .../scala/org/apache/spark/mllib/linalg/MatricesSuite.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala index 57fbb78c4b85c..e60ca67ff28ab 100644 --- a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala +++ b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala @@ -82,11 +82,11 @@ class MatricesSuite extends SparkFunSuite { val dm2 = Matrices.dense(2, 2, Array(0.0, 2.0, 1.0, 3.0)) assert(dm1 === dm2.transpose) - val sm1 = dm1.toSparse + val sm1 = dm1.asInstanceOf[DenseMatrix].toSparse assert(sm1 === sm1) assert(sm1 !== sm1.transpose) - val sm2 = dm2.toSparse + val sm2 = dm2.asInstanceOf[DenseMatrix].toSparse assert(sm1 === sm2.transpose) } From 22782df742b43800773637ffd3af86f15d7d8966 Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Mon, 10 Aug 2015 13:54:48 -0700 Subject: [PATCH 4/6] Add equality based on matrix semantics, not representation --- .../main/scala/org/apache/spark/mllib/linalg/Matrices.scala | 5 +++-- .../scala/org/apache/spark/mllib/linalg/MatricesSuite.scala | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala index bae90c787a4c1..cf2d8a8ad1393 100644 --- a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala +++ b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala @@ -257,7 +257,7 @@ class DenseMatrix( this(numRows, numCols, values, false) override def equals(o: Any): Boolean = o match { - case m: DenseMatrix => + case m: Matrix => m.numRows == numRows && m.numCols == numCols && Arrays.equals(toArray, m.toArray) case _ => false } @@ -520,7 +520,8 @@ class SparseMatrix( values: Array[Double]) = this(numRows, numCols, colPtrs, rowIndices, values, false) override def equals(o: Any): Boolean = o match { - case m: SparseMatrix => toBreeze == m.toBreeze + case m: Matrix => + m.numRows == numRows && m.numCols == numCols && Arrays.equals(toArray, m.toArray) case _ => false } diff --git a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala index e60ca67ff28ab..bfd6d5495f5e0 100644 --- a/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala +++ b/mllib/src/test/scala/org/apache/spark/mllib/linalg/MatricesSuite.scala @@ -84,10 +84,12 @@ class MatricesSuite extends SparkFunSuite { val sm1 = dm1.asInstanceOf[DenseMatrix].toSparse assert(sm1 === sm1) + assert(sm1 === dm1) assert(sm1 !== sm1.transpose) val sm2 = dm2.asInstanceOf[DenseMatrix].toSparse assert(sm1 === sm2.transpose) + assert(sm1 === dm2.transpose) } test("matrix copies are deep copies") { From ab6f3c8f748d803245f596b3412b293cb3012522 Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Mon, 10 Aug 2015 17:16:45 -0700 Subject: [PATCH 5/6] Sparse matrix compare for equals --- .../main/scala/org/apache/spark/mllib/linalg/Matrices.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala index cf2d8a8ad1393..206a09e045a49 100644 --- a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala +++ b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala @@ -520,8 +520,7 @@ class SparseMatrix( values: Array[Double]) = this(numRows, numCols, colPtrs, rowIndices, values, false) override def equals(o: Any): Boolean = o match { - case m: Matrix => - m.numRows == numRows && m.numCols == numCols && Arrays.equals(toArray, m.toArray) + case m: Matrix => toBreeze == m.toBreeze case _ => false } From bb70d5e77d35e711910eac7ee28f78cc09e7e07b Mon Sep 17 00:00:00 2001 From: Feynman Liang Date: Tue, 11 Aug 2015 11:14:56 -0700 Subject: [PATCH 6/6] Breeze compare for dense matrices as well, in case other is sparse --- .../main/scala/org/apache/spark/mllib/linalg/Matrices.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala index 206a09e045a49..1139ce36d50b8 100644 --- a/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala +++ b/mllib/src/main/scala/org/apache/spark/mllib/linalg/Matrices.scala @@ -257,8 +257,7 @@ class DenseMatrix( this(numRows, numCols, values, false) override def equals(o: Any): Boolean = o match { - case m: Matrix => - m.numRows == numRows && m.numCols == numCols && Arrays.equals(toArray, m.toArray) + case m: Matrix => toBreeze == m.toBreeze case _ => false }