Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-5503][MLLIB] Example code for Power Iteration Clustering #4495

Closed
wants to merge 10 commits into from
Closed

[SPARK-5503][MLLIB] Example code for Power Iteration Clustering #4495

wants to merge 10 commits into from

Conversation

javadba
Copy link
Contributor

@javadba javadba commented Feb 10, 2015

No description provided.

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27180 has started for PR 4495 at commit efeec45.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27180 has finished for PR 4495 at commit efeec45.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27182 has started for PR 4495 at commit f7ff43d.

  • This patch merges cleanly.

mengxr
mengxr reviewed Feb 10, 2015
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/mllib/PowerIterationClusteringExample.scala
import org.apache.spark.mllib.clustering.PowerIterationClustering
import org.apache.spark.rdd.RDD
import org.apache.spark.{SparkConf, SparkContext}
import scopt.OptionParser
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

organize imports (You can use this plugin: https://plugins.jetbrains.com/plugin/7350)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had already manually fixed the order according to your email. Maybe I did optimize imports again: since the above shows the order was reset .. Apparently the default behavior in IJ is completely different than the spark requirements. Will try that plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That plugin is apparently for IJ13 and says should not be used. So that is a bit confusing.. anyways I did some more manual rearranging.

@mengxr
Copy link
Contributor

mengxr commented Feb 10, 2015

@javadba Please also check the hidden inline comments.

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27185 has started for PR 4495 at commit cef28f4.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27182 has finished for PR 4495 at commit f7ff43d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class Params(

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27185 has finished for PR 4495 at commit cef28f4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class Params(

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27228 has started for PR 4495 at commit d7ac350.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 10, 2015

Test build #27228 has finished for PR 4495 at commit d7ac350.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

@mengxr
Copy link
Contributor

mengxr commented Feb 10, 2015

@javadba I still want to decrease the code complexity because this is example code, so users can focus on the PIC part of the code but not on how to prepare the input. There are two things we can change:

  1. ask users to input the size of the smallest circle instead of total number of points
  2. do not treat the radius of smallest circle differently
  3. remove similarity and use gaussianSimilarity directly

@SparkQA
Copy link

SparkQA commented Feb 11, 2015

Test build #27245 has started for PR 4495 at commit 2878675.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 11, 2015

Test build #27245 has finished for PR 4495 at commit 2878675.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

mengxr
mengxr reviewed Feb 11, 2015
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/mllib/PowerIterationClusteringExample.scala
val parser = new OptionParser[Params]("PIC Circles") {
head("PowerIterationClusteringExample: an example PIC app using concentric circles.")
opt[Int]('k', "k")
.text(s"number of circles (/clusters), default: ${defaultParams.k}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still use 2-space indentation here. 4-indentation only applies to method/class declarations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err yes.. back to the drawing board for the IJ auto formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi I have created a Youtrack issue for Intellij for this formatting limitation:

https://youtrack.jetbrains.com/issue/IDEA-136404

@SparkQA
Copy link

SparkQA commented Feb 11, 2015

Test build #27306 has started for PR 4495 at commit 3c84b14.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Feb 12, 2015

Test build #27306 has finished for PR 4495 at commit 3c84b14.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

asfgit pushed a commit that referenced this pull request Feb 13, 2015
@sboeschhuawei @mengxr
[SPARK-5503][MLLIB] Example code for Power Iteration Clustering
5e63942 
Author: sboeschhuawei <[email protected]>

Closes #4495 from javadba/picexamples and squashes the following commits:

3c84b14 [sboeschhuawei] PIC Examples updates from Xiangrui's comments round 5
2878675 [sboeschhuawei] Fourth round with xiangrui on PICExample
d7ac350 [sboeschhuawei] Updates to PICExample from Xiangrui's comments round 3
d7f0cba [sboeschhuawei] Updates to PICExample from Xiangrui's comments round 3
cef28f4 [sboeschhuawei] Further updates to PICExample from Xiangrui's comments
f7ff43d [sboeschhuawei] Update to PICExample from Xiangrui's comments
efeec45 [sboeschhuawei] Update to PICExample from Xiangrui's comments
03e8de4 [sboeschhuawei] Added PICExample
c509130 [sboeschhuawei] placeholder for pic examples
5864d4a [sboeschhuawei] placeholder for pic examples

(cherry picked from commit e1a1ff8)
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Feb 13, 2015

LGTM. Merged into master and branch-1.3. Thanks!!

@asfgit asfgit closed this in e1a1ff8 Feb 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@javadba @SparkQA @AmplabJenkins @mengxr @sboeschhuawei