Repository: scikit-learn/scikit-learn
Pull request: Logistic lgbf verbosity
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs

Fixes #11519 

<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->


#### What does this implement/fix? Explain your changes.

Maps verbose to specific values of iprint used in optimizer


#### Any other comments?


<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
LogisticRegression verbosity broken for l-bfgs (and newton-cg?)
Setting verbose=100 in LogisticRegression results in nothing being printed.
There is some conversion between verbose and iprint, and somewhere things in there are going wrong.

FYI this prints to stdout, so if you're using Jupyter, look at your console, not the Jupyter output.

Hints from issue discussion:
I'm happy to work on this. The issue is `iprint=(verbose > 0) - 1`, which causes the only option to be iprint of -1 or 0. 

What is the desired behavior?  Setting it `iprint=verbose -1` makes sense, but then setting verbose=100 would mean printing every 99th iteration. Is that what is wanted?
higher verbose should mean more messages, while higher iprint means less messages, right? I found the docstring of iprint a bit confusing tbh.
There's probably no direct way to translate verbose to iprint, but my desired behavior would be that for verbose>0 it prints the message at convergence, for verbose>1 it also prints some intermediate messages. Whether we want finer grained verbosity levels (like =2 prints every 100 iterations or >2 prints every iteration) I'm not sure. But might be nice?
Right, so maybe map the verbose to iprint as

```
0 -> -1
1 -> 50
2 -> 1
3 -> 100
>4 -> 101
```
yes, I think that is what makes sense? wow iprint is really not confusing at all...
Yeah, one thing about the above is that it skips the iprint option of `0`, which is not no verbosity.... could add that too, but that is just a lot of verbose options.

Also, I am new to contributing. I am not sure when to make a PR. I currently have that set up on my branch here:
https://github.com/dillongardner/scikit-learn/tree/logistic-lgbf-verbosity
now is good ;)

Commit message: Removed accidental test file

Patch to review:
diff --git a/sklearn/linear_model/logistic.py b/sklearn/linear_model/logistic.py
index e4ea696ce7146..a2c8fddb0a653 100644
--- a/sklearn/linear_model/logistic.py
+++ b/sklearn/linear_model/logistic.py
@@ -711,10 +711,20 @@ def logistic_regression_path(X, y, pos_class=None, Cs=10, fit_intercept=True,
     n_iter = np.zeros(len(Cs), dtype=np.int32)
     for i, C in enumerate(Cs):
         if solver == 'lbfgs':
+            if  0 < verbose <= 1:
+                iprint = 50
+            elif 1 < verbose <= 2:
+                iprint = 1
+            elif 2 < verbose <= 3:
+                iprint = 100
+            elif verbose > 3:
+                iprint = 101
+            else:
+                iprint = -1
             w0, loss, info = optimize.fmin_l_bfgs_b(
                 func, w0, fprime=None,
                 args=(X, target, 1. / C, sample_weight),
-                iprint=(verbose > 0) - 1, pgtol=tol, maxiter=max_iter)
+                iprint=iprint, pgtol=tol, maxiter=max_iter)
             if info["warnflag"] == 1:
                 warnings.warn("lbfgs failed to converge. Increase the number "
                               "of iterations.", ConvergenceWarning)


Code review comments:
diff_hunk: @@ -711,10 +711,20 @@ def logistic_regression_path(X, y, pos_class=None, Cs=10, fit_intercept=True,
     n_iter = np.zeros(len(Cs), dtype=np.int32)
     for i, C in enumerate(Cs):
         if solver == 'lbfgs':
+            if  0 < verbose <= 1:
+                iprint = 50
+            elif 1 < verbose <= 2:
+                iprint = 1
path: sklearn/linear_model/logistic.py
text: @user1:
reading the doc https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.fmin_l_bfgs_b.html it seems iprint=99 is also a specific option.

@author:
Right. Every value between 0 and 101 is a valid option with difference output. I think the question is, given how verbose should match the higher to more output, how best to map verbose to iprint. The current behavior only allows two options (iprint=-1 and iprint=0). The proposed change is to allow some more verbosity but not as much granularity.

Do you think it is preferred to provide the full granularity of iprint? If so, could map
```
if verbose <= 0:
    iprint = -1
elif 0 < verbose < 100:
    iprint = (100 - verbose)
else:
  iprint = verbose
```

This would provide full access to the range of output from iprint, but much the higher verbose = more output


@user1:
I like option you proposed here so far so verbose=0,1,2,3 etc like we do everywhere.