[MRG] Added penalty='none' to LogisticRegression #12860

NicolasHug · 2018-12-24T20:07:42Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Support 'none' for penalty in LogisticRegerssion which is equivalent to setting penalty='l2' and C=np.inf.

This is supported by all solvers except liblinear which seems to take forever even on small datasets. For the other solvers, I haven't observed any significant change (in fit time or score) by using C=np.inf instead of the default value (see plots)

Any other comments?

Benchmarks on my laptop (8Go RAM, i5 7th gen) showing logloss and fit time in seconds, averaged over 10 experiments:

setting C to np.inf:

setting C to default value:

I don't know if I did something wrong but the docs say that sag should be faster for large datasets while lbfgs should be slower, but I'm observing the reverse.

from collections import defaultdict
from time import time
import warnings

import numpy as np
from sklearn.datasets import make_classification
from sklearn.linear_model import LogisticRegression
from sklearn.exceptions import ConvergenceWarning
import statsmodels.api as sm
from sklearn.metrics import log_loss


# logit = sm.Logit(y, X)
# res = logit.fit()
# print(log_loss(y, res.predict(X)))

def return_default_dict_list():
    return defaultdict(list)

solvers = ('lbfgs', 'newton-cg', 'sag', 'saga')

n_exp = 10
n_samples_list = [int(x) for x in (1e2, 1e3, 1e4, 5e4, 1e5, 5e5, 1e6)]
durations = defaultdict(return_default_dict_list)
scores = defaultdict(return_default_dict_list)
conv_warn = defaultdict(return_default_dict_list)

for n_samples in n_samples_list:
    print(n_samples)
    for exp in range(n_exp):
        X, y = make_classification(n_samples=n_samples)  # no random state
        for solver in solvers:
            with warnings.catch_warnings(record=True) as ws:
                print(solver)
                tic = time()
                lr = LogisticRegression(C=np.inf, solver=solver, random_state=0)
                lr.fit(X, y)
                duration = time() - tic
                print(f'fit duration: {duration:.3f}s')
                score = log_loss(y, lr.predict_proba(X))
                print(f'logloss: {score}')

                if any(issubclass(w.category, ConvergenceWarning) for w in ws):
                    conv_warn[n_samples][solver].append(True)
                else:
                    conv_warn[n_samples][solver].append(False)

                durations[n_samples][solver].append(duration)
                scores [n_samples][solver].append(score)


import matplotlib.pyplot as plt

fig, axs = plt.subplots(2)

for solver in solvers:
    avg_duration = [np.mean(durations[n_samples][solver]) for n_samples in n_samples_list]
    axs[0].plot(n_samples_list, avg_duration, label=solver)
    axs[0].set_ylabel('duration (s)')

    avg_score = [np.mean(scores[n_samples][solver]) for n_samples in n_samples_list]
    axs[1].plot(n_samples_list, avg_score, label=solver)
    axs[1].set_ylabel('log_loss')

for ax in (axs):
    ax.set_xscale('log')
    ax.set_xlabel('n_samples')
    ax.legend()

plt.show()

jnothman · 2019-01-08T07:11:15Z

sklearn/linear_model/logistic.py

        Used to specify the norm used in the penalization. The 'newton-cg',
        'sag' and 'lbfgs' solvers support only l2 penalties. 'elasticnet' is
-        only supported by the 'saga' solver.
+        only supported by the 'saga' solver. If 'none' (not supported by the
+        liblinear solver), no regularization is applied: this is equivalent


Perhaps don't bother stating the equivalence here? There is enough to read

jnothman · 2019-01-08T07:15:43Z

sklearn/linear_model/logistic.py

@@ -1705,10 +1731,12 @@ class LogisticRegressionCV(LogisticRegression, BaseEstimator,
        l2 penalty with liblinear solver. Prefer dual=False when
        n_samples > n_features.

-    penalty : str, 'l1', 'l2', or 'elasticnet', optional (default='l2')
+    penalty : str, 'l1', 'l2', 'elasticnet' or 'none', optional (default='l2')


What's the point of supporting none in CV, when its role is to determine the optimal C under cross-validation?

Good point, I was in auto pilot ^^

jnothman

Please test the error msg in LRCV(penalty='none'). Ideally, mention that 'none' is not useful w/ LRCV

jnothman · 2019-01-08T21:58:05Z

sklearn/linear_model/logistic.py

-        only supported by the 'saga' solver. If 'none' (not supported by the
-        liblinear solver), no regularization is applied: this is equivalent
-        to setting C to ``np.inf`` with 'l2'.
+        only supported by the 'saga' solver.


Perhaps note that 'none' is not useful with LogisticRegressionCV?

jnothman

Thanks!

jnothman · 2019-01-09T22:25:46Z

doc/whats_new/v0.21.rst

@@ -88,6 +88,12 @@ Support for Python 3.4 and below has been officially dropped.
  :class:`linear_model.LogisticRegressionCV` now support Elastic-Net penalty,
  with the 'saga' solver. :issue:`11646` by :user:`Nicolas Hug <NicolasHug>`.

+- |Feature| :class:`linear_model.LogisticRegression` now supports an


I suspect that since we're not adding any new functionality, this should strictly be an enhancement.

jnothman · 2019-01-09T22:26:45Z

Thanks!

…12860)" This reverts commit 8adbddb.

NicolasHug added 3 commits December 24, 2018 14:57

Added penalty='none' to LogisticRegression and LogisticRegressionCV

dcad60d

Updated whatsnew

49beba4

Fixed futurewarning

4725e2b

agramfort approved these changes Jan 7, 2019

View reviewed changes

jnothman reviewed Jan 8, 2019

View reviewed changes

NicolasHug added 2 commits January 8, 2019 10:20

Merge branch 'master' into logistic_reg_no_regu

8a7576d

Addressed comments

818567c

NicolasHug changed the title ~~[MRG] Added penalty='none' to LogisticRegression and LogisticRegressionCV~~ [MRG] Added penalty='none' to LogisticRegression Jan 8, 2019

fixed flake8

842d5e3

jnothman reviewed Jan 8, 2019

View reviewed changes

Added error message for penalty=none in LRCV

5bdb23d

jnothman approved these changes Jan 9, 2019

View reviewed changes

Update v0.21.rst

eb162ab

jnothman merged commit 0a07364 into scikit-learn:master Jan 9, 2019

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

ENH Added penalty='none' to LogisticRegression (scikit-learn#12860)

8adbddb

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "ENH Added penalty='none' to LogisticRegression (scikit-learn#…

0391f5f

…12860)" This reverts commit 8adbddb.

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "ENH Added penalty='none' to LogisticRegression (scikit-learn#…

a130711

…12860)" This reverts commit 8adbddb.

amueller mentioned this pull request Jun 11, 2019

add section on regularization to logistic regression docs #14070

Closed

koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019

ENH Added penalty='none' to LogisticRegression (scikit-learn#12860)

8f2dcee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[MRG] Added penalty='none' to LogisticRegression #12860

[MRG] Added penalty='none' to LogisticRegression #12860

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[MRG] Added penalty='none' to LogisticRegression #12860

[MRG] Added penalty='none' to LogisticRegression #12860

Uh oh!

Conversation

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!