[SPARK-33043][ML] Handle spark.driver.maxResultSize=0 in RowMatrix heuristic computation #29925
Conversation
| require(aggregatedObjectSizeInBytes > 0, | ||
| "Cannot compute aggregate depth heuristic based on a zero-size object to aggregate") | ||
|
|
||
| val maxDriverResultSizeInBytes = rows.conf.get[Long](MAX_RESULT_SIZE) | ||
| if (maxDriverResultSizeInBytes == 0) { | ||
| // Unlimited result size, so 1 is OK | ||
| return 1 |
There was a problem hiding this comment.
Out of curiosity, why is this 1 given that the default argument for depth in rdd.treeAggregate is 2?
There was a problem hiding this comment.
Good question - 2 could be OK too. I suspect that was chosen to line up with the default max result size. Higher depths are needed when the result size is smaller, so I figured when the result size is unlimited, the depth can be as low as possible, 1.
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129315 has finished for PR 29925 at commit
|
mllib/src/main/scala/org/apache/spark/mllib/linalg/distributed/RowMatrix.scala
Outdated
Show resolved
Hide resolved
mllib/src/test/scala/org/apache/spark/mllib/linalg/distributed/RowMatrixSuite.scala
Outdated
Show resolved
Hide resolved
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129348 has finished for PR 29925 at commit
|
|
Merged to master/3.0 |
…uristic computation ### What changes were proposed in this pull request? RowMatrix contains a computation based on spark.driver.maxResultSize. However, when this value is set to 0, the computation fails (log of 0). The fix is simply to correctly handle this setting, which means unlimited result size, by using a tree depth of 1 in the RowMatrix method. ### Why are the changes needed? Simple bug fix to make several Spark ML functions which use RowMatrix run correctly in this case. ### Does this PR introduce _any_ user-facing change? Not other than the bug fix of course. ### How was this patch tested? Existing RowMatrix tests plus a new test. Closes #29925 from srowen/SPARK-33043. Authored-by: Sean Owen <srowen@gmail.com> Signed-off-by: Sean Owen <srowen@gmail.com> (cherry picked from commit f86171a) Signed-off-by: Sean Owen <srowen@gmail.com>
…uristic computation ### What changes were proposed in this pull request? RowMatrix contains a computation based on spark.driver.maxResultSize. However, when this value is set to 0, the computation fails (log of 0). The fix is simply to correctly handle this setting, which means unlimited result size, by using a tree depth of 1 in the RowMatrix method. ### Why are the changes needed? Simple bug fix to make several Spark ML functions which use RowMatrix run correctly in this case. ### Does this PR introduce _any_ user-facing change? Not other than the bug fix of course. ### How was this patch tested? Existing RowMatrix tests plus a new test. Closes apache#29925 from srowen/SPARK-33043. Authored-by: Sean Owen <srowen@gmail.com> Signed-off-by: Sean Owen <srowen@gmail.com> (cherry picked from commit f86171a) Signed-off-by: Sean Owen <srowen@gmail.com>
What changes were proposed in this pull request?
RowMatrix contains a computation based on spark.driver.maxResultSize. However, when this value is set to 0, the computation fails (log of 0). The fix is simply to correctly handle this setting, which means unlimited result size, by using a tree depth of 1 in the RowMatrix method.
Why are the changes needed?
Simple bug fix to make several Spark ML functions which use RowMatrix run correctly in this case.
Does this PR introduce any user-facing change?
Not other than the bug fix of course.
How was this patch tested?
Existing RowMatrix tests plus a new test.