[Spark-4060] [MLlib] exposing special rdd functions to the public

[numbnut]

No description provided.

[AmplabJenkins]

Can one of the admins verify this patch?

[srowen]

At best, this would become an "Experimental" API and marked as such, and need a unit test or two more maybe. What's the use case that makes it worth committing to support these externally?

[mengxr]

@srowen Unit tests are in

https://github.com/numbnut/spark/blob/master/mllib/src/test/scala/org/apache/spark/mllib/rdd/RDDFunctionsSuite.scala

I think we can mark it @DeveloperApi. I'm a little concerned about the return type of sliding, which is not Java-friendly. Any suggestions?

[srowen]

Cool, you would know best if it's ready for external use. Looks good on unit tests.

What if it returned RDD[Array[T]]? I experimented briefly with making that change and it looked like it would work out fairly simple. That's Java friendlier?

[mengxr]

Sounds good. @numbnut Could you update the PR and change the following?

1. add @DeveloperAPI to RDDFunctions
2. change the return type of sliding to RDD[Array[T]] and update the code in other places

Thanks!

[javadba]

RE: use case. We are considering to use the treeAggregate function within the implementation of SpectralClustering. In addition it was noted that the EigenvalueDecomposition.symmetricEigs is private: it is likely we would like to use that too

[shivaram]

Yes - treeAggregate is very useful -- In fact I was going to suggest moving it to the core RDD API. Any reasons to not do that ?

[numbnut]

I updated the pull request like proposed. Please review it carefully because I'm new to Spark and Scala.

[mengxr]

@shivaram For common RDD operations in core/sql, each task is small (including the result) and there are more partitions than executors. treeAggregate creates a shuffle stage and holds data there, while aggregate can start working when partial results are available.

@pwendell Are you comfortable with adding those RDD functions to core?

[shivaram]

Agree that using aggregate vs. treeAggregate depends on the computation, reduction function -- but I don't think its specific to MLLib per se. Any Spark application that has CPU intensive code can benefit from treeAggregate. My view is that we shouldn't replace aggregate with this -- we should just allow users to choose the right aggregation strategy based on what they need

[mengxr]

Does it work if we still leave the class as private[mllib]?

[numbnut]

In the tests it works fine. I just wanted to expose as less as possible.
Shall I replace "private[mllib]" with "@DeveloperAPI"?

[mengxr]

I think it is necessary to replace private[mllib] with @DeveloperApi. Otherwise, the methods under RDDFunctions won't show up in the generated doc.

[mengxr]

ok to test

[mengxr]

test this please

[SparkQA]

Test build #22784 has started for PR 2907 at commit 0840e6e.

• This patch merges cleanly.

[SparkQA]

Test build #22784 has finished for PR 2907 at commit 0840e6e.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class RDDFunctions[T: ClassTag](self: RDD[T]) extends Serializable

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22784/
Test FAILed.

[SparkQA]

Test build #22875 has started for PR 2907 at commit 7f7c767.

• This patch merges cleanly.

[numbnut]

I'm sorry for breaking the tests. I thought they had run cleanly on my machine but I found the mistake and corrected it. Can't explain that.

I also changed "private[mllib]" to "@DeveloperAPI" to make it visible in the docs.

Am I supposed to rebase to the branch-1.2 or what can I do to simplify the merge process?

[SparkQA]

Test build #22875 has finished for PR 2907 at commit 7f7c767.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class RDDFunctions[T: ClassTag](self: RDD[T]) extends Serializable

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22875/
Test PASSed.

[mengxr]

LGTM. There is a minor issue with @DeveloperAPI annotation, where we also need :: DeveloperApi :: at the beginning of the doc. I will fix that later. I've merged this into master and branch-1.2. Thanks!