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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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,
53 matches
Mail list logo