[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-29 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/12560 --- 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-29 Thread MLnick
Github user MLnick commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-215732275 LGTM. Merged to 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-28 Thread MLnick
Github user MLnick commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r61384326 --- Diff: mllib/src/main/scala/org/apache/spark/ml/util/Instrumentation.scala --- @@ -85,6 +85,13 @@ private[ml] class Instrumentation[E <: Estimator[_]]

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-27 Thread yanboliang
Github user yanboliang commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-215301738 This looks good to me, 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-27 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-215235728 @thunterdb What do you think about our discussions? Thanks! Miao --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-27 Thread MLnick
Github user MLnick commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-215168378 I'd go with what we have here for now. We could revisit later if necessary. If a user really wants to check the number of user/item factors they can do a count on the

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-27 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-215135795 @MLnick @yanboliang Any further comments? Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214913414 Test PASSed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214913413 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-21491 **[Test build #57042 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57042/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214904807 **[Test build #57042 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57042/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread thunterdb
Github user thunterdb commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214890439 In any case, we should also log the success with `instrLog.logSuccess(model)` --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread thunterdb
Github user thunterdb commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r61166736 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,6 +395,10 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-26 Thread thunterdb
Github user thunterdb commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214890157 @wangmiao1981 sorry for the delay. I agree with @MLnick that the developer API of ALS is already brittle enough as it stands. One way to still do it is to add one

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214574485 Test PASSed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214574484 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214574399 **[Test build #56945 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56945/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214569258 **[Test build #56945 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56945/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214515243 @MLnick I agree. I will remove the feature log now and only log parameters. I will keep the named feature method. --- If your project is set up for it, you can

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread MLnick
Github user MLnick commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214490075 @wangmiao1981 I tend to agree with @yanboliang. I don't think adding instrumentation is critical enough to break the `train` method signature, even if it is

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-25 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-214474647 @MLnick Yanbo does not like the change of train() API. The new parameter is optional, so the user of train should not be aware of this change. In addition, I

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213704898 Test PASSed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213704897 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213704837 **[Test build #56792 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56792/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213688248 **[Test build #56792 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56792/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213683707 Test FAILed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213683704 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213683674 **[Test build #56790 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56790/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-23 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213681560 **[Test build #56790 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56790/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-22 Thread wangmiao1981
Github user wangmiao1981 commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60701976 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -607,7 +611,8 @@ object ALS extends DefaultParamsReadable[ALS] with

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-22 Thread yanboliang
Github user yanboliang commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60696182 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -607,7 +611,8 @@ object ALS extends DefaultParamsReadable[ALS] with

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213268198 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213268088 **[Test build #56655 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56655/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213268202 Test FAILed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213264908 **[Test build #56655 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56655/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213264393 retest 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213164023 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213164024 Test FAILed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213164000 **[Test build #56609 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56609/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213162291 **[Test build #56609 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56609/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213059111 @thunterdb train method has count information, but it will change the signature of the train method. I am learning how to avoid collect and changing signature.

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread wangmiao1981
Github user wangmiao1981 commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213023729 Thanks all for your comments! Let me figure out how to collect the information without slowing the algorithm. @MLnick The names are passed to the log. For

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread thunterdb
Github user thunterdb commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-213009688 @wangmiao1981 thanks for the PR. Can you see if you could make it work without having to evaluate the input data? --- If your project is set up for it, you can

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread thunterdb
Github user thunterdb commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60616196 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,13 +395,21 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread thunterdb
Github user thunterdb commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60615798 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,13 +395,21 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread MLnick
Github user MLnick commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60542344 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,13 +395,21 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread MLnick
Github user MLnick commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60542203 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,13 +395,21 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread holdenk
Github user holdenk commented on a diff in the pull request: https://github.com/apache/spark/pull/12560#discussion_r60531928 --- Diff: mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala --- @@ -395,13 +395,21 @@ class ALS(@Since("1.4.0") override val uid: String)

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-212763813 Test PASSed. Refer to this link for build results (access rights to CI server needed):

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-212763807 Merged build finished. 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

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-212763564 **[Test build #56484 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56484/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-20 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12560#issuecomment-212752513 **[Test build #56484 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56484/consoleFull)** for PR 12560 at commit

[GitHub] spark pull request: [SPARK-14571][ML]Log instrumentation in ALS

2016-04-20 Thread wangmiao1981
GitHub user wangmiao1981 opened a pull request: https://github.com/apache/spark/pull/12560 [SPARK-14571][ML]Log instrumentation in ALS ## What changes were proposed in this pull request? Add log instrumentation for parameters: rank, numUserBlocks, numItemBlocks,