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.

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

Solver - add a wrapper for scipy L-BFGS solver #165

Merged
merged 13 commits into from
Jun 12, 2023

Conversation

Badr-MOUFAD
Copy link
Collaborator

@Badr-MOUFAD Badr-MOUFAD commented Jun 12, 2023

This adds a BFGS to the solvers' toolbox of skglm.
It proceeds as follows:

  • add wrapper around scipy.optimize.minimize(method="L-BFGS-B")
  • unitest against scikit-learn L2-Logistic regression

skglm/solvers/bfgs.py Outdated Show resolved Hide resolved
@Badr-MOUFAD Badr-MOUFAD changed the title Solver - add a wrapper for scipy BFGS solver Solver - add a wrapper for scipy L-BFGS solver Jun 12, 2023
@mathurinm
Copy link
Collaborator

Can you run it quickly on moderate size datasets in https://github.com/benchopt/benchmark_logreg_l2 ?

doc/api.rst Outdated Show resolved Hide resolved

Parameters
----------
max_iter : int, default 20
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is quite small, is it enough on a 1000 x 1000 dataset for example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

50?

@Badr-MOUFAD
Copy link
Collaborator Author

Can you run it quickly on moderate size datasets in https://github.com/benchopt/benchmark_logreg_l2 ?

Here are the benchmark figures of skglm lbfgs vs. scikit-learn lbfgs

@mathurinm
Copy link
Collaborator

Can you run it quickly on moderate size datasets in https://github.com/benchopt/benchmark_logreg_l2 ?

Here are the benchmark figures of skglm lbfgs vs. scikit-learn lbfgs

I can't see the iterations on it, how many iterations does it take to converge?
On some datasets there seem to be a very small difference in final objective value (1e-6), it's not blocking but do you know the cause?

@mathurinm mathurinm merged commit 0e5c938 into scikit-learn-contrib:main Jun 12, 2023
@Badr-MOUFAD
Copy link
Collaborator Author

My apologies, I performed the benchmark using an old version of benchopt.

Here the new figures.

On some datasets there seem to be a very small difference in final objective value (1e-6), it's not blocking but do you know the cause?

Several parameters determine the convergence of scipy L-BFGS-B, among which ftol. In the new benchmark I set it to zero so that to account only for max_iter. With that being applied, we get an objective slightly below scikit-learn LBFGS

Should we fix that, @mathurinm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants