You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Lewuathe <gi...@git.apache.org> on 2015/03/15 12:23:08 UTC
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
GitHub user Lewuathe opened a pull request:
https://github.com/apache/spark/pull/5033
[SPARK-6336] LBFGS should document what convergenceTol means
LBFGS uses convergence tolerance. This value should be written document as an argument.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Lewuathe/spark SPARK-6336
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5033.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5033
----
commit 6ccb304a426a2f622e0dcbda02565774daa0ec3b
Author: lewuathe <le...@me.com>
Date: 2015-03-15T11:20:47Z
[SPARK-6336] LBFGS should document what convergenceTol means
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-81708980
**[Test build #28648 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28648/consoleFull)** for PR 5033 at commit [`ac03c3a`](https://github.com/apache/spark/commit/ac03c3af0bd14450c8acd5679034487b96f2d41c) after a configured wait of `120m`.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-82543020
Merged into master and branch-1.3. 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 have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-80970195
[Test build #28625 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28625/consoleFull) for PR 5033 at commit [`6ccb304`](https://github.com/apache/spark/commit/6ccb304a426a2f622e0dcbda02565774daa0ec3b).
* This patch merges cleanly.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-82368412
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28722/
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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-81008356
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28625/
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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-81008343
**[Test build #28625 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28625/consoleFull)** for PR 5033 at commit [`6ccb304`](https://github.com/apache/spark/commit/6ccb304a426a2f622e0dcbda02565774daa0ec3b) after a configured wait of `120m`.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by Lewuathe <gi...@git.apache.org>.
Github user Lewuathe commented on a diff in the pull request:
https://github.com/apache/spark/pull/5033#discussion_r26568664
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/optimization/LBFGS.scala ---
@@ -60,6 +60,8 @@ class LBFGS(private var gradient: Gradient, private var updater: Updater)
/**
* Set the convergence tolerance of iterations for L-BFGS. Default 1E-4.
* Smaller value will lead to higher accuracy with the cost of more iterations.
+ * This value must be nonnegative. Lower convergence tolerance is less tolerant
--- End diff --
I'm sorry for my unnatural expression. I modified to be more natural. Thank you.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5033
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-82368329
[Test build #28722 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28722/consoleFull) for PR 5033 at commit [`e738b33`](https://github.com/apache/spark/commit/e738b333aa84f9a0c8733ed6401e9343d4555cd0).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-81709017
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28648/
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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-81635864
[Test build #28648 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28648/consoleFull) for PR 5033 at commit [`ac03c3a`](https://github.com/apache/spark/commit/ac03c3af0bd14450c8acd5679034487b96f2d41c).
* This patch merges cleanly.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/5033#discussion_r26565348
--- Diff: mllib/src/main/scala/org/apache/spark/mllib/optimization/LBFGS.scala ---
@@ -60,6 +60,8 @@ class LBFGS(private var gradient: Gradient, private var updater: Updater)
/**
* Set the convergence tolerance of iterations for L-BFGS. Default 1E-4.
* Smaller value will lead to higher accuracy with the cost of more iterations.
+ * This value must be nonnegative. Lower convergence tolerance is less tolerant
--- End diff --
Nit: works better as "Lower convergence values are less tolerant and therefore ..." as you have elsewhere. (I can get to this on merge if you don't first.) LGTM otherwise. I think we can ignore the Jenkins timeout. I will verify the docs build before merging.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5033#issuecomment-82324475
[Test build #28722 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28722/consoleFull) for PR 5033 at commit [`e738b33`](https://github.com/apache/spark/commit/e738b333aa84f9a0c8733ed6401e9343d4555cd0).
* This patch merges cleanly.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6336] LBFGS should document what conver...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/5033#discussion_r26448937
--- Diff: docs/mllib-optimization.md ---
@@ -203,6 +203,9 @@ regularization, as well as L2 regularizer.
recommended.
* `maxNumIterations` is the maximal number of iterations that L-BFGS can be run.
* `regParam` is the regularization parameter when using regularization.
+* `convergenceTol` designates the end of iterations based on relative comparison
--- End diff --
If linking to Breeze, would be better to link straight to its LBFGS. However I don't know if we want to promise that implementation detail.
Should we also be updating the javadoc for `LBFGS.scala` and other similar places where this param is documented?
I'm not sure this text helps someone understand what the value is or how to set it -- what are adjusted values? You're right, it's fairly opaque beyond being something relative (rather than something that might need to change with the input). The thing to know is that lower values mean more iterations and the value must be nonnegative. So how about something more like:
Controls how much relative change is still allowed when L-BFGS is considered to converge. Must be nonnegative. Lower values are less tolerant and therefore generally cause more iterations to be run.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org