Skip to content

Conversation

@BaiGang
Copy link
Contributor

@BaiGang BaiGang commented Jun 17, 2014

https://issues.apache.org/jira/browse/SPARK-2163

This pull request includes the change for [SPARK-2163]:

  • Changed the convergence tolerance parameter from type Int to type Double.
  • Added types for vars in class LBFGS, making the style consistent with class GradientDescent.
  • Added associated test to check that optimizing via class LBFGS produces the same results as via calling runLBFGS from object LBFGS.

This is a very minor change but it will solve the problem in my implementation of a regression model for count data, where I make use of LBFGS for parameter estimation.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

Choose a reason for hiding this comment

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

In most of the mllib codebase, we don't specify the type of variable. Can you remove them?

@BaiGang
Copy link
Contributor Author

BaiGang commented Jun 18, 2014

@dbtsai Thanks for looking into this PR. I've updated the code, removing the unnecessary type specifications for the vars in class LBFGS.

Also I have a question. If I externally (outside mllib) make uses of LBFGS, should I use the object method LBFGS.runLBFGS(.) or create an instance of class LBFGS and run the optimize(.) method? Which one of these is the supposed proper usage of the LBFGS optimizer? As for the tests, I think we should just test against that usage.

@dbtsai
Copy link
Member

dbtsai commented Jun 18, 2014

I think it's legacy reason to have two different way to access the API. As far as I know, @mengxr is working on consolidating the interface. He probably can talk about more on this topic.

@BaiGang
Copy link
Contributor Author

BaiGang commented Jun 18, 2014

Thanks, @dbtsai .
Just did a git rebase. Will wait for the conclusion of consolidated interfaces.

@mengxr
Copy link
Contributor

mengxr commented Jun 18, 2014

Jenkins, test this please.

Copy link
Contributor

Choose a reason for hiding this comment

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

two space indentation

@mengxr
Copy link
Contributor

mengxr commented Jun 18, 2014

The change looks good to me. Let us wait for Jenkins and MIMA.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@dbtsai
Copy link
Member

dbtsai commented Jun 18, 2014

I think it will be a problem for MIMA to change the signature.

@AmplabJenkins
Copy link

Merged build finished.

@AmplabJenkins
Copy link

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15870/

…type Double. For the reason and the problem caused by an Int parameter, please check https://issues.apache.org/jira/browse/SPARK-2163. Added a test in LBFGSSuite for validating that optimizing via class LBFGS produces the same results as calling runLBFGS from object LBFGS. Keep the indentations and styles correct.
@mengxr
Copy link
Contributor

mengxr commented Jun 18, 2014

Jenkins, add to whitelist.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15884/

@mengxr
Copy link
Contributor

mengxr commented Jun 20, 2014

Merged. Thanks!

@asfgit asfgit closed this in d484dde Jun 20, 2014
asfgit pushed a commit that referenced this pull request Jun 20, 2014
https://issues.apache.org/jira/browse/SPARK-2163

This pull request includes the change for **[SPARK-2163]**:

* Changed the convergence tolerance parameter from type `Int` to type `Double`.
* Added types for vars in `class LBFGS`, making the style consistent with `class GradientDescent`.
* Added associated test to check that optimizing via `class LBFGS` produces the same results as via calling `runLBFGS` from `object LBFGS`.

This is a very minor change but it will solve the problem in my implementation of a regression model for count data, where I make use of LBFGS for parameter estimation.

Author: Gang Bai <[email protected]>

Closes #1104 from BaiGang/fix_int_tol and squashes the following commits:

cecf02c [Gang Bai] Changed setConvergenceTol'' to specify tolerance with a parameter of type Double. For the reason and the problem caused by an Int parameter, please check https://issues.apache.org/jira/browse/SPARK-2163. Added a test in LBFGSSuite for validating that optimizing via class LBFGS produces the same results as calling runLBFGS from object LBFGS. Keep the indentations and styles correct.

(cherry picked from commit d484dde)
Signed-off-by: Xiangrui Meng <[email protected]>
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
https://issues.apache.org/jira/browse/SPARK-2163

This pull request includes the change for **[SPARK-2163]**:

* Changed the convergence tolerance parameter from type `Int` to type `Double`.
* Added types for vars in `class LBFGS`, making the style consistent with `class GradientDescent`.
* Added associated test to check that optimizing via `class LBFGS` produces the same results as via calling `runLBFGS` from `object LBFGS`.

This is a very minor change but it will solve the problem in my implementation of a regression model for count data, where I make use of LBFGS for parameter estimation.

Author: Gang Bai <[email protected]>

Closes apache#1104 from BaiGang/fix_int_tol and squashes the following commits:

cecf02c [Gang Bai] Changed setConvergenceTol'' to specify tolerance with a parameter of type Double. For the reason and the problem caused by an Int parameter, please check https://issues.apache.org/jira/browse/SPARK-2163. Added a test in LBFGSSuite for validating that optimizing via class LBFGS produces the same results as calling runLBFGS from object LBFGS. Keep the indentations and styles correct.
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
https://issues.apache.org/jira/browse/SPARK-2163

This pull request includes the change for **[SPARK-2163]**:

* Changed the convergence tolerance parameter from type `Int` to type `Double`.
* Added types for vars in `class LBFGS`, making the style consistent with `class GradientDescent`.
* Added associated test to check that optimizing via `class LBFGS` produces the same results as via calling `runLBFGS` from `object LBFGS`.

This is a very minor change but it will solve the problem in my implementation of a regression model for count data, where I make use of LBFGS for parameter estimation.

Author: Gang Bai <[email protected]>

Closes apache#1104 from BaiGang/fix_int_tol and squashes the following commits:

cecf02c [Gang Bai] Changed setConvergenceTol'' to specify tolerance with a parameter of type Double. For the reason and the problem caused by an Int parameter, please check https://issues.apache.org/jira/browse/SPARK-2163. Added a test in LBFGSSuite for validating that optimizing via class LBFGS produces the same results as calling runLBFGS from object LBFGS. Keep the indentations and styles correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants