You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hhbyyh <gi...@git.apache.org> on 2015/01/12 06:23:09 UTC
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
GitHub user hhbyyh opened a pull request:
https://github.com/apache/spark/pull/3997
[SPARK-5186] [MLLIB] Vector.equals and Vector.hashCode are very inefficient
JIRA Issue: https://issues.apache.org/jira/browse/SPARK-5186
Currently SparseVector is using the inherited equals from Vector, which will create a full-size array for even the sparse vector. The pull request contains a specialized equals optimization that improves on both time and space.
1. The implementation will be consistent with the original. Especially it will keep equality comparison between SparseVector and DenseVector.
2. For the hash code, overriding it may generate some breaking change and we should do it in another PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hhbyyh/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3997.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3997
----
commit 574114433222f4adb95e1e98f5d96e72e733eb4d
Author: Yuhao Yang <yu...@yuhaodevbox.sh.intel.com>
Date: 2015-01-13T04:31:13Z
Specialized equals for SparseVector
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23136069
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/linalg/VectorsSuite.scala ---
@@ -89,6 +89,29 @@ class VectorsSuite extends FunSuite {
}
}
+ test("sparse equals with explicit 0") {
+ val sv1 = Vectors.sparse(10, Array(1, 3, 4, 7), Array(0.9, 0.8, 0.7, 0.6))
+ val sv2 = Vectors.sparse(10, Array(0, 1, 3, 4, 7), Array(0.0, 0.9, 0.8, 0.7, 0.6))
+ val sv3 = Vectors.sparse(10, Array(1, 3, 4, 5, 6, 7), Array(0.9, 0.8, 0.7, 0, 0, 0.6))
+ val sv4 = Vectors.sparse(10, Array(1, 3, 4, 7, 9), Array(0.9, 0.8, 0.7, 0.6, 0))
+ val sv5 = Vectors.sparse(10, Array(0, 1, 2, 3, 4, 5, 6, 7, 8, 9),
+ Array(0, 0.9, 0, 0.8, 0.7, 0, 0, 0.6, 0, 0))
+
+ val vectors = Seq(sv1, sv2, sv3, sv4, sv5)
+
+ for (v <- vectors; u <- vectors) {
+ assert(v === u)
+ assert(v.## === u.##)
+ }
+
+ val another = Vectors.sparse(10, Array(1, 3, 4, 7), Array(0.9, 0.2, 0.7, 0.6))
+
+ for (v <- vectors) {
+ assert(v != another)
+ assert(v.## != another.##)
+ }
+ }
+
--- End diff --
This still needs tests involving dense vectors right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70398840
[Test build #25714 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25714/consoleFull) for PR 3997 at commit [`93f0d46`](https://github.com/apache/spark/commit/93f0d461487f9582a6bc2a34f09179dbe8672d3d).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22799128
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
--- End diff --
Thanks @srowen for the comment. Glad to discuss it with someone.
_Vector: override def hashCode(): Int = util.Arrays.hashCode(this.toArray)_
I understand it's the general guideline to override `hashCode` at the same time.
Yet intentionally or not, the original code promises that `DenseVector` and `SparseVector` would return the same results of `equals` and `hashCode` for the same array content. And that makes some senses.
As in the description of the PR, I don’t want to introduce breaking changes. And if we want to keep the original design, the current implementation of `hashCode` in `Vector` is one of the best choices. That’s why `hashCode` was intentionally left out of the PR. (maybe I should add some comment)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh closed the pull request at:
https://github.com/apache/spark/pull/3997
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69535209
[Test build #25399 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25399/consoleFull) for PR 3997 at commit [`5741144`](https://github.com/apache/spark/commit/574114433222f4adb95e1e98f5d96e72e733eb4d).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70226850
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25645/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3997
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70732700
[Test build #25843 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25843/consoleFull) for PR 3997 at commit [`0d9d130`](https://github.com/apache/spark/commit/0d9d13040e4d2730ec1c8ceaf5d8d48ead9d0bd8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23087101
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -57,7 +57,19 @@ sealed trait Vector extends Serializable {
}
}
- override def hashCode(): Int = util.Arrays.hashCode(this.toArray)
+ override def hashCode(): Int = {
+ var result: Int = size + 31
+ this.foreachActive{case(index, value) =>
--- End diff --
You should put spaces around the `{` and in `if(...` below. I think you could note where this hash function comes from (the JDK right?) to explain it. But yes this is looking right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23090557
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +461,37 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector => {
+ if (this.size != v.size) { return false }
+ val thisValues = this.values
+ val thisIndices = this.indices
+ val thisSize = thisValues.size
+ val otherValues = v.values
+ val otherIndices = v.indices
+ val otherSize = otherValues.size
+
+ var k1 = 0
+ var k2 = 0
+ var allEqual = true
+ while (allEqual) {
+ while (k1 < thisSize && thisValues(k1) == 0) k1 += 1
+ while (k2 < otherSize && otherValues(k2) == 0) k2 += 1
+
+ if (k1 >= thisSize || k2 >= otherSize) {
+ return k1 >= thisSize && k2 >= otherSize // check end alignment
+ }
+ allEqual = thisIndices(k1) == otherIndices(k2) && thisValues(k1) == otherValues(k2)
+ k1 += 1
+ k2 += 1
+ }
+ allEqual
+ }
+ case _ => super.equals(other)
+ }
+ }
+
--- End diff --
You mean a function structured like `sqdist`, not a distance-based function right? yes that sounds good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70107710
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25607/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70400053
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25714/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22783753
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
--- End diff --
You must override `hashCode` too!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70744145
[Test build #25843 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25843/consoleFull) for PR 3997 at commit [`0d9d130`](https://github.com/apache/spark/commit/0d9d13040e4d2730ec1c8ceaf5d8d48ead9d0bd8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23088157
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -57,7 +57,19 @@ sealed trait Vector extends Serializable {
}
}
- override def hashCode(): Int = util.Arrays.hashCode(this.toArray)
+ override def hashCode(): Int = {
+ var result: Int = size + 31
+ this.foreachActive{case(index, value) =>
--- End diff --
Noted. Yes it's from JDK. I'll add comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70453518
[Test build #25739 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25739/consoleFull) for PR 3997 at commit [`0d9d130`](https://github.com/apache/spark/commit/0d9d13040e4d2730ec1c8ceaf5d8d48ead9d0bd8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70217074
@hhbyyh @srowen There are some performance issues if we use unnecessary index lookup. Having many `other.values(i)` calls is slower than `val otherValues = other.values` and then many `otherValues(i)` calls. I'm suggesting some code like the following (I didn't try compiling the code):
~~~scala
var ii0 = this.indices
var vv0 = this.values
var ii1 = this.indices
var vv1 = this.values
var j0 = 0
var j1 = 0
var i0 = 0
var i1 = 0
var v0 = 0.0
var v1 = 0.0
var pj0 = -1
var pj1 = -1
var allEqual = true
while(allEqual && j0 < sz0 && j1 < sz1) {
if (pj0 < j0) {
i0 = ii0(j0)
v0 = vv0(j0)
pj0 = j0
}
if (pj1 < j1) {
i1 = ii1(j1)
v1 = vv1(j1)
pj1 = j1
}
if (i0 == i1) {
allEqual &&= v0 == v1
j0 += 1
j1 += 1
} else if (i0 < i1) {
allEqual &&= v0 == 0.0
j0 += 1
} else {
allEqual &&= v1 == 0.0
j1 += 1
}
while (allEqual & j0 < sz0) {
allEqual &&= vv0(j0) == 0.0
j0 += 1
}
while (allEqual & j1 < sz1) {
allEqual &&= vv1(j1) == 0.0
j1 += 1
}
allEqual
~~~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70205338
@hhbyyh Oops yes that's a typo. The helper function should refer to the local argument `values` only!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70755279
LGTM. Merged into master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69535881
Thanks, can someone help review please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22800634
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
--- End diff --
Yeah I was thinking of performance too. That is a fair point of course about not changing the semantics. I think the `hashCode` impl could be changed in both places to produce the same result while being faster for sparse vectors: what if the hash were defined only over nonzero entries? I think `equals` could likewise be sped up also for the case of comparing sparse to dense? that is I think this can be taken a step further than just optimizing sparse == sparse.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70107705
[Test build #25607 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25607/consoleFull) for PR 3997 at commit [`50abef3`](https://github.com/apache/spark/commit/50abef35ef4ccb4f4f037bb7d29c5200cc7ab7cb).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22908168
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector =>
+ this.size == v.size &&
+ util.Arrays.equals(this.indices, v.indices) &&
+ util.Arrays.equals(this.values, v.values)
--- End diff --
Nice suggestion. This should be your contribution and I'll close the PR. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70270369
@mengxr I did some further test and find the perf difference caused by the heap access. Thanks for pointing it out.
Sent an update containing the locality improvement. @srowen you may notice I changed the implementation back a little and uses the allEqual inspired by @mengxr :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70744149
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25843/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70280970
[Test build #25663 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25663/consoleFull) for PR 3997 at commit [`985e160`](https://github.com/apache/spark/commit/985e16088196c2fb808c63c67ac7ee420edb4efa).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23089433
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +461,37 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector => {
+ if (this.size != v.size) { return false }
+ val thisValues = this.values
+ val thisIndices = this.indices
+ val thisSize = thisValues.size
+ val otherValues = v.values
+ val otherIndices = v.indices
+ val otherSize = otherValues.size
+
+ var k1 = 0
+ var k2 = 0
+ var allEqual = true
+ while (allEqual) {
+ while (k1 < thisSize && thisValues(k1) == 0) k1 += 1
+ while (k2 < otherSize && otherValues(k2) == 0) k2 += 1
+
+ if (k1 >= thisSize || k2 >= otherSize) {
+ return k1 >= thisSize && k2 >= otherSize // check end alignment
+ }
+ allEqual = thisIndices(k1) == otherIndices(k2) && thisValues(k1) == otherValues(k2)
+ k1 += 1
+ k2 += 1
+ }
+ allEqual
+ }
+ case _ => super.equals(other)
+ }
+ }
+
--- End diff --
Yes, about the sparse vs dense.
My current thought is put a new function in `Vectors`, maybe something like `sqdist(sparse, dense)`, and call it from `Vector.equals`, does it sound good?
Yes, about the sparse vs dense.
My current thought is put a new function in `Vectors`, maybe something like `sqdist(sparse, dense)`, and call it from `Vector.equals`, does it sound good?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22815763
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
--- End diff --
+1 on @srowen 's suggestion. We can design a hashing scheme based on the size and nonzero entries (with their indices). But I think we could do that in a separate PR. Does it sound good?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70508158
New updated added for ut update and function name change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70114222
[Test build #25608 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25608/consoleFull) for PR 3997 at commit [`a6952c3`](https://github.com/apache/spark/commit/a6952c39532594e1f4eb1c2f764d528420320ea8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70127522
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25608/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70402015
Sent an update unifying the "sparse vs sparse" and "sparse vs dense".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70109130
Just send an update. I didn't use`sqdist` due to the performance concern. Since the original equals is actually a fail-fast comparison, yet `sqdist` will inevitably compute through the vectors even if the first element is different. The performance will be hard to accept for scenarios like doc2Vec over a large vocabulary.
Current implementation is still based on the comparison for indices and values, just with the handling of the explicit 0. I gave some tests to the implementation and add a few ut. Any comment will be welcome!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70028461
Agree with @srowen ! @hhbyyh It would be great if you want to re-open this PR and implement a faster and correct `Vector.equals`. If `sqdist` doesn't check vector sizes, we should add checks there. It is also nice to add a unit test in `VectorsSuite` to show it works for two sparse vectors with different `values` array but equal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69531100
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69949365
@hhbyyh Although I don't wish to speak for @mengxr I strongly suspect he would welcome you implementing the idea. I'm not sure we should use `sqdist` since it looks like it doesn't require vectors of the same size, and that seems essential for `equals` and `hashCode`? But I think it can indeed handle the cases of sparse vs dense similarly and efficiently.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23139673
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -51,13 +51,35 @@ sealed trait Vector extends Serializable {
override def equals(other: Any): Boolean = {
other match {
- case v: Vector =>
- util.Arrays.equals(this.toArray, v.toArray)
+ case v2: Vector => {
+ if (this.size != v2.size) return false
+ (this, v2) match {
+ case (s1: SparseVector, s2: SparseVector) =>
+ Vectors.getEquality(s1.indices, s1.values, s2.indices, s2.values)
+ case (s1: SparseVector, d1: DenseVector) =>
+ Vectors.getEquality(s1.indices, s1.values, 0 until d1.size, d1.values)
+ case (d1: DenseVector, s1: SparseVector) =>
+ Vectors.getEquality(0 until d1.size, d1.values, s1.indices, s1.values)
+ case (_, _) => util.Arrays.equals(this.toArray, v2.toArray)
--- End diff --
Thanks for review, @srowen
Yes, I can change the name. Just like the name in `util.Arrays.equals`.
And for the catch-all case, I was thinking `util.Arrays.equals` is also fail-fast and it can cover the dense vs dense case well, and with some extensiblity. Maybe I don't catch your point. Let me know if still we should change this. Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70224754
Oh I didn't see your comment before the PR. @mengxr I surely found a lot of the code using the pattern (first assign locally, then access) in Spark, and we can follow it.
Just to be honest, I'm not sure I completely understand the reason, is it for sparing a memory addressing? I ran some local perf test and got no obvious difference. Would you please share some insight? Thanks a lot!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70449484
[Test build #25739 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25739/consoleFull) for PR 3997 at commit [`0d9d130`](https://github.com/apache/spark/commit/0d9d13040e4d2730ec1c8ceaf5d8d48ead9d0bd8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70453523
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25739/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23027089
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,31 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector => {
+ if (this.size != v.size) { return false }
+ var k1 = 0
+ var k2 = 0
+ while (true) {
+ while (k1 < this.values.size && this.values(k1) == 0) k1 += 1
+ while (k2 < v.values.size && v.values(k2) == 0) k2 += 1
+
+ if (k1 == this.values.size || k2 == v.values.size) {
+ return (k1 == this.values.size && k2 == v.values.size) // check end alignment
+ }
+ if (this.indices(k1) != v.indices(k2) || this.values(k1) != v.values(k2)) {
+ return false
+ }
+ k1 += 1
+ k2 += 1
+ }
+ throw new Exception("unreachable")
--- End diff --
I wondered to myself whether this could be simplified to not have `while (true)`, the dummy `Exception`, etc. The best I could do was with a helper function:
```
...
var k1 = nextNonzero(this.values, 0)
var k2 = nextNonzero(v.values, 0)
while (k1 < this.values.size && k2 < v.values.size) {
if (this.indices(k1) != v.indices(k2) || this.values(k1) != v.values(k2)) {
return false
}
k1 = nextNonzero(this.values, k1 + 1)
k2 = nextNonzero(v.values, k2 + 1)
}
return (k1 == this.values.size && k2 == v.values.size)
...
def nextNonzero(values: Array[Double], from: Int): Int = {
var index = from
while (index < this.values.size && this.values(index) == 0.0) index += 1
index
}
```
I'm not sure it's better, just food for thought.
So the idea would be to specialize `hashCode` as well, and also handle `DenseVector` right? and even remove the implementations in the parent?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
GitHub user hhbyyh reopened a pull request:
https://github.com/apache/spark/pull/3997
[SPARK-5186] [MLLIB] Vector.equals and Vector.hashCode are very inefficient
JIRA Issue: https://issues.apache.org/jira/browse/SPARK-5186
Currently SparseVector is using the inherited equals from Vector, which will create a full-size array for even the sparse vector. The pull request contains a specialized equals optimization that improves on both time and space.
1. The implementation will be consistent with the original. Especially it will keep equality comparison between SparseVector and DenseVector.
2. For the hash code, overriding it may generate some breaking change and we should do it in another PR.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hhbyyh/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3997.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3997
----
commit 574114433222f4adb95e1e98f5d96e72e733eb4d
Author: Yuhao Yang <yu...@yuhaodevbox.sh.intel.com>
Date: 2015-01-13T04:31:13Z
Specialized equals for SparseVector
commit f41b135ab0394e881bd03c87bb02aa77be61fb64
Author: Yuhao Yang <hh...@gmail.com>
Date: 2015-01-16T15:13:12Z
iterative equals for sparse vector
commit 50abef35ef4ccb4f4f037bb7d29c5200cc7ab7cb
Author: Yuhao Yang <hh...@gmail.com>
Date: 2015-01-16T15:47:19Z
fix ut for sparse vector with explicit 0
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22893260
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector =>
+ this.size == v.size &&
+ util.Arrays.equals(this.indices, v.indices) &&
+ util.Arrays.equals(this.values, v.values)
--- End diff --
Just realized that this did change the behavior of the original `equals`, because we allow `values` to have explicit zeros. Maybe we can use `Vectors.sqdist(this, other) == 0` to implement `equals()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70127518
[Test build #25608 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25608/consoleFull) for PR 3997 at commit [`a6952c3`](https://github.com/apache/spark/commit/a6952c39532594e1f4eb1c2f764d528420320ea8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r22838761
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +449,16 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
--- End diff --
Yes, the nonzero entries idea did cross my mind. Maybe it's overcautious that I think it might become a complexity if we want to have another kind of Vector in the future, which don't have handy internal structure to scan for the nonzero entries. Again, this can be overcautious.
And the dense == sparse idea looks good, maybe that suits into a util method better as it would not introduce the existence of `DenseVector` to `SparseVector` and vice versa.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69535216
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25399/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70226841
[Test build #25645 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25645/consoleFull) for PR 3997 at commit [`bdf8789`](https://github.com/apache/spark/commit/bdf8789b5ee33eb0bda465e68cc31892b049c381).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70269269
[Test build #25663 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25663/consoleFull) for PR 3997 at commit [`985e160`](https://github.com/apache/spark/commit/985e16088196c2fb808c63c67ac7ee420edb4efa).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70400050
[Test build #25714 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25714/consoleFull) for PR 3997 at commit [`93f0d46`](https://github.com/apache/spark/commit/93f0d461487f9582a6bc2a34f09179dbe8672d3d).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70732131
test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70280988
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25663/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23136064
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -51,13 +51,35 @@ sealed trait Vector extends Serializable {
override def equals(other: Any): Boolean = {
other match {
- case v: Vector =>
- util.Arrays.equals(this.toArray, v.toArray)
+ case v2: Vector => {
+ if (this.size != v2.size) return false
+ (this, v2) match {
+ case (s1: SparseVector, s2: SparseVector) =>
+ Vectors.getEquality(s1.indices, s1.values, s2.indices, s2.values)
+ case (s1: SparseVector, d1: DenseVector) =>
+ Vectors.getEquality(s1.indices, s1.values, 0 until d1.size, d1.values)
+ case (d1: DenseVector, s1: SparseVector) =>
+ Vectors.getEquality(0 until d1.size, d1.values, s1.indices, s1.values)
+ case (_, _) => util.Arrays.equals(this.toArray, v2.toArray)
--- End diff --
Yeah looking good. I have only minor things to say, which don't necessarily need to change. For example the utility method could be called `Vectors.equals`. Should there even be a catch-all case? I wonder if failing fast is good, if no other implementations are expected now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69532014
add to whitelist
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69532019
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70107537
[Test build #25607 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25607/consoleFull) for PR 3997 at commit [`50abef3`](https://github.com/apache/spark/commit/50abef35ef4ccb4f4f037bb7d29c5200cc7ab7cb).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23125961
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +461,37 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector => {
+ if (this.size != v.size) { return false }
+ val thisValues = this.values
+ val thisIndices = this.indices
+ val thisSize = thisValues.size
+ val otherValues = v.values
+ val otherIndices = v.indices
+ val otherSize = otherValues.size
+
+ var k1 = 0
+ var k2 = 0
+ var allEqual = true
+ while (allEqual) {
+ while (k1 < thisSize && thisValues(k1) == 0) k1 += 1
+ while (k2 < otherSize && otherValues(k2) == 0) k2 += 1
+
+ if (k1 >= thisSize || k2 >= otherSize) {
+ return k1 >= thisSize && k2 >= otherSize // check end alignment
+ }
+ allEqual = thisIndices(k1) == otherIndices(k2) && thisValues(k1) == otherValues(k2)
+ k1 += 1
+ k2 += 1
+ }
+ allEqual
+ }
+ case _ => super.equals(other)
+ }
+ }
+
--- End diff --
yes, and I found the sparse vs dense is actually quite similar to sparse vs sparse, I'm trying to unify them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70029156
Thanks, I'll try on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70220103
[Test build #25645 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25645/consoleFull) for PR 3997 at commit [`bdf8789`](https://github.com/apache/spark/commit/bdf8789b5ee33eb0bda465e68cc31892b049c381).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70767616
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23087164
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/Vectors.scala ---
@@ -449,6 +461,37 @@ class SparseVector(
override def toString: String =
"(%s,%s,%s)".format(size, indices.mkString("[", ",", "]"), values.mkString("[", ",", "]"))
+ override def equals(other: Any): Boolean = {
+ other match {
+ case v: SparseVector => {
+ if (this.size != v.size) { return false }
+ val thisValues = this.values
+ val thisIndices = this.indices
+ val thisSize = thisValues.size
+ val otherValues = v.values
+ val otherIndices = v.indices
+ val otherSize = otherValues.size
+
+ var k1 = 0
+ var k2 = 0
+ var allEqual = true
+ while (allEqual) {
+ while (k1 < thisSize && thisValues(k1) == 0) k1 += 1
+ while (k2 < otherSize && otherValues(k2) == 0) k2 += 1
+
+ if (k1 >= thisSize || k2 >= otherSize) {
+ return k1 >= thisSize && k2 >= otherSize // check end alignment
+ }
+ allEqual = thisIndices(k1) == otherIndices(k2) && thisValues(k1) == otherValues(k2)
+ k1 += 1
+ k2 += 1
+ }
+ allEqual
+ }
+ case _ => super.equals(other)
+ }
+ }
+
--- End diff --
Yes I like this version. Still pretty clear, and I think it's fine to optimize this a bit. There's still an opportunity to optimize sparse vs dense comparison, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69849296
There's a better idea from mengxr.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-70198699
@srowen Thanks a lot for the improvement. The code do look cleaner and the helper function `nextNonzero` can also be used in the `hashCode` function. Great idea.
I'll use it if you don't mind. And `this` in the `nextNonzero` can be removed, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3997#issuecomment-69532197
[Test build #25399 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25399/consoleFull) for PR 3997 at commit [`5741144`](https://github.com/apache/spark/commit/574114433222f4adb95e1e98f5d96e72e733eb4d).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5186] [MLLIB] Vector.equals and Vector....
Posted by hhbyyh <gi...@git.apache.org>.
Github user hhbyyh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3997#discussion_r23139716
--- Diff: mllib/src/test/scala/org/apache/spark/mllib/linalg/VectorsSuite.scala ---
@@ -89,6 +89,29 @@ class VectorsSuite extends FunSuite {
}
}
+ test("sparse equals with explicit 0") {
+ val sv1 = Vectors.sparse(10, Array(1, 3, 4, 7), Array(0.9, 0.8, 0.7, 0.6))
+ val sv2 = Vectors.sparse(10, Array(0, 1, 3, 4, 7), Array(0.0, 0.9, 0.8, 0.7, 0.6))
+ val sv3 = Vectors.sparse(10, Array(1, 3, 4, 5, 6, 7), Array(0.9, 0.8, 0.7, 0, 0, 0.6))
+ val sv4 = Vectors.sparse(10, Array(1, 3, 4, 7, 9), Array(0.9, 0.8, 0.7, 0.6, 0))
+ val sv5 = Vectors.sparse(10, Array(0, 1, 2, 3, 4, 5, 6, 7, 8, 9),
+ Array(0, 0.9, 0, 0.8, 0.7, 0, 0, 0.6, 0, 0))
+
+ val vectors = Seq(sv1, sv2, sv3, sv4, sv5)
+
+ for (v <- vectors; u <- vectors) {
+ assert(v === u)
+ assert(v.## === u.##)
+ }
+
+ val another = Vectors.sparse(10, Array(1, 3, 4, 7), Array(0.9, 0.2, 0.7, 0.6))
+
+ for (v <- vectors) {
+ assert(v != another)
+ assert(v.## != another.##)
+ }
+ }
+
--- End diff --
sure. I'll add some.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org