You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mbittmann <gi...@git.apache.org> on 2015/02/06 20:15:04 UTC
[GitHub] spark pull request: [SPARK-5656] Fail gracefully for large values ...
GitHub user mbittmann opened a pull request:
https://github.com/apache/spark/pull/4433
[SPARK-5656] Fail gracefully for large values of k and/or n that will ex...
...ceed max int.
Large values of k and/or n in EigenValueDecomposition.symmetricEigs will result in array initialization to a value larger than Integer.MAX_VALUE in the following: var v = new Array[Double](n * ncv)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mbittmann/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4433.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 #4433
----
commit a604816b25988f1200758b65a3ae15efbb684de7
Author: bittmannm <ma...@agilex.com>
Date: 2015-02-06T19:12:51Z
[SPARK-5656] Fail gracefully for large values of k and/or n that will exceed max int.
Large values of k and/or n in EigenValueDecomposition.symmetricEigs will result in array initialization to a value larger than Integer.MAX_VALUE in the following: var v = new Array[Double](n * ncv)
----
---
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-5656] Fail gracefully for large values ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24274900
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -66,6 +66,9 @@ private[mllib] object EigenValueDecomposition {
// ncv must be smaller than n
val ncv = math.min(2 * k, n)
+ require(ncv * n.toLong < Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
Seems reasonable, though I have a few minor suggestions. I suppose `<=` is OK, technically? and if this size is worth checking, it's probably worth checking `ncv * (ncv + 8)` later. And then, maybe it's best to keep the check by the allocation so they don't get out of sync if someone changes them later.
---
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-5656] Fail gracefully for large values ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73332912
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26972/
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-5656] Fail gracefully for large values ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24280703
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -79,6 +79,10 @@ private[mllib] object EigenValueDecomposition {
// Mode 1: A*x = lambda*x, A symmetric
iparam(6) = 1
+ require(ncv * n.toLong <= Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
Sorry, one other tiny thing: the problematic product is not necessarily 2*k*n here. I think you could simplify these error messages to just say that the k and n are too large, rather than explain the computation. Right now the expressions are repeated 3 places. Might be nice to keep that to 2, and keep them together.
---
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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73342722
[Test build #26977 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26977/consoleFull) for PR 4433 at commit [`e49cbbb`](https://github.com/apache/spark/commit/e49cbbb0f9e0aa23c7f74bcf59df5c6967d6f5c0).
* 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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73332910
[Test build #26972 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26972/consoleFull) for PR 4433 at commit [`860836b`](https://github.com/apache/spark/commit/860836be46f737ea6fe1494b3f006d7ed5240d22).
* 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-5656] Fail gracefully for large values ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73302682
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26929/
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-5656] Fail gracefully for large values ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24289433
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -79,6 +79,11 @@ private[mllib] object EigenValueDecomposition {
// Mode 1: A*x = lambda*x, A symmetric
iparam(6) = 1
+ require(n * ncv.toLong <= Integer.MAX_VALUE, "Large n and/or k will exceed " +
--- End diff --
I don't mean to belabor this, but the new second error message implies that k alone is the problem, and that k exceeds 2^31-1, and those aren't necessarily true. Concretely I'm suggesting something simple like
```
require(n * ncv.toLong <= Integer.MAX_VALUE && ncv * (ncv.toLong + 8) <= Integer.MAX_VALUE,
s"k = $k and/or n = $n are too large to compute an eigendecomposition")
```
---
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-5656] Fail gracefully for large values ...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73296645
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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73297460
[Test build #26929 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26929/consoleFull) for PR 4433 at commit [`a604816`](https://github.com/apache/spark/commit/a604816b25988f1200758b65a3ae15efbb684de7).
* 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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73332803
[Test build #26972 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26972/consoleFull) for PR 4433 at commit [`860836b`](https://github.com/apache/spark/commit/860836be46f737ea6fe1494b3f006d7ed5240d22).
* 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-5656] Fail gracefully for large values ...
Posted by mbittmann <gi...@git.apache.org>.
Github user mbittmann commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24282208
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -79,6 +79,10 @@ private[mllib] object EigenValueDecomposition {
// Mode 1: A*x = lambda*x, A symmetric
iparam(6) = 1
+ require(ncv * n.toLong <= Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
Okay, I think i figured out what you are saying. Take a look. The check on k may be overkill, since by definition it must be less than n. Therefore k*n is always greater than k*k. There could always be that weird scenario though. Thanks for the help.
---
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-5656] Fail gracefully for large values ...
Posted by mbittmann <gi...@git.apache.org>.
Github user mbittmann commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24280548
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -66,6 +66,9 @@ private[mllib] object EigenValueDecomposition {
// ncv must be smaller than n
val ncv = math.min(2 * k, n)
+ require(ncv * n.toLong < Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
Thanks for the feedback. I committed your suggestions into this PR.
---
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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73302672
[Test build #26929 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26929/consoleFull) for PR 4433 at commit [`a604816`](https://github.com/apache/spark/commit/a604816b25988f1200758b65a3ae15efbb684de7).
* This patch **fails Spark unit 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-5656] Fail gracefully for large values ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73342729
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26977/
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-5656] Fail gracefully for large values ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24281733
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -79,6 +79,10 @@ private[mllib] object EigenValueDecomposition {
// Mode 1: A*x = lambda*x, A symmetric
iparam(6) = 1
+ require(ncv * n.toLong <= Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
You could, sure. Yes, I'm saying the caller may just need or want to know that, basically, k or n is too big. It would be simpler and easier than maintaining a longer explanation.
---
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-5656] Fail gracefully for large values ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73369177
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27002/
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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73366284
[Test build #27002 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27002/consoleFull) for PR 4433 at commit [`ee56e05`](https://github.com/apache/spark/commit/ee56e05644bbb39883828b2eff58af5e0a4df309).
* 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-5656] Fail gracefully for large values ...
Posted by mbittmann <gi...@git.apache.org>.
Github user mbittmann commented on a diff in the pull request:
https://github.com/apache/spark/pull/4433#discussion_r24281478
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/linalg/EigenValueDecomposition.scala ---
@@ -79,6 +79,10 @@ private[mllib] object EigenValueDecomposition {
// Mode 1: A*x = lambda*x, A symmetric
iparam(6) = 1
+ require(ncv * n.toLong <= Integer.MAX_VALUE, "Product of 2*k*n must be smaller than " +
--- End diff --
Just want to confirm. Are you suggesting just a single 'require' statement that will AND the two conditions? Then just provide a generic comment that either n or k are two large?
---
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-5656] Fail gracefully for large values ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4433
---
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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73369173
[Test build #27002 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27002/consoleFull) for PR 4433 at commit [`ee56e05`](https://github.com/apache/spark/commit/ee56e05644bbb39883828b2eff58af5e0a4df309).
* 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-5656] Fail gracefully for large values ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73337348
[Test build #26977 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26977/consoleFull) for PR 4433 at commit [`e49cbbb`](https://github.com/apache/spark/commit/e49cbbb0f9e0aa23c7f74bcf59df5c6967d6f5c0).
* 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-5656] Fail gracefully for large values ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4433#issuecomment-73295691
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