8000 [MRG] Better convergence warnings for lbfgs solver in LogisticRegression by rth · Pull Request #11767 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG] Better convergence warnings for lbfgs solver in LogisticRegression #11767

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[MRG] Better convergence warnings for lbfgs solver in LogisticRegression #11767

wants to merge 1 commit into from

Conversation

rth
Copy link
Member
@rth rth commented Aug 7, 2018 •
edited
Loading

When the lbfgs solver in LogisticRegression fails to converge, the resulting ConvergenceWarning is not very informative. This PR increases it's verbosity so we have a better estimation of how bad the convergence is.
This is particularly relevant if lbfgs is to become the default solver (#11476)

The tricky part is that, as far as I understood, scipy.optimize.fmin_l_bfgs_b only returns the evaluated gradient at the minimum, while the convergence criterion is the max |projected gradient|. Still IMO providing at least some information about the final gradient is better than nothing..

Example

from sklearn.linear_model import LogisticRegression
from sklearn.datasets import load_iris


iris = load_iris()

estimator = LogisticRegression(solver='lbfgs', multi_class='ovr', max_iter=20)
estimator.fit(iris.data, iris.target)

Output on master

sklearn/linear_model/logistic.py:723: ConvergenceWarning: lbfgs failed to converge. Increase the number of iterations.
  "of iterations.", ConvergenceWarning)
sklearn/linear_model/logistic.py:723: ConvergenceWarning: lbfgs failed to converge. Increase the number of iterations.
  "of iterations.", ConvergenceWarning)
sklearn/linear_model/logistic.py:723: ConvergenceWarning: lbfgs failed to converge. Increase the number of iterations.
  "of iterations.", ConvergenceWarning)

Output with this PR

sklearn/linear_model/logistic.py:734: ConvergenceWarning: lbfgs failed to converge with max_iter=20. max(|grad|) = 1.610e+00 while pgtol=1.000e-04 (see scipy.optimize.fmin_l_bfgs_b documentation for more information). Increase the number of iterations.
  ConvergenceWarning)
sklearn/linear_model/logistic.py:734: ConvergenceWarning: lbfgs failed to converge with max_iter=20. max(|grad|) = 4.764e+00 while pgtol=1.000e-04 (see scipy.optimize.fmin_l_bfgs_b documentation for more information). Increase the number of iterations.
  ConvergenceWarning)
sklearn/linear_model/logistic.py:734: ConvergenceWarning: lbfgs failed to converge with max_iter=20. max(|grad|) = 4.413e-01 while pgtol=1.000e-04 (see scipy.optimize.fmin_l_bfgs_b documentation for more information). Increase the number of iterations.

@rth
Copy link
Member Author
rth commented Aug 7, 2018

(Circle Ci fails due to unrelated mldata download issues)

@jnothman
Copy link
Member
jnothman commented Aug 7, 2018 via email

@rth
Copy link
Member Author
rth commented Aug 13, 2018

It's probably not very important, but say in a long running system with some log files, the case of the almost converged training (a gradient of a few 1e-3) in some cases, one might not bother retraining. For a moderately low gradient one could indeed increase the number of iterations. If the final gradient is > few 1.0-10.0 even after a significant number of iterations, it may suggest that something is wrong about the pipeline.

I guess generally, I was just generally itchy about an optimization problem that returned "convergence failed, try again" without outputting any useful information to understand what's happening. But maybe the problems considered here are sufficiently nicely convex that there is not reason to worry about it.

Still, something like this can help spot potential issues (related to #11536).

@rth
Copy link
Member Author
rth commented Sep 26, 2018

There doesn't seem too much enthusiasm about this PR. There are larger issues to address anyway. If someone is interested please comment or re-open.

@rth rth closed this Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rth @jnothman
0