[MRG+1] ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' by jnothman · Pull Request #11905 · scikit-learn/scikit-learn

jnothman · 2018-08-24T03:18:52Z

Adds multi_class='auto'

        If the option chosen is 'ovr', then a binary problem is fit for each
        label. For 'multinomial' the loss minimised is the multinomial loss fit
        across the entire probability distribution, *even when the data is
        binary*. 'multinomial' is unavailable when solver='liblinear'.
        'auto' selects 'ovr' if the data is binary, or if solver='liblinear',
        and otherwise selects 'multinomial'.

'auto' will be default from 0.22. Also sets solver='lbfgs' to default from 0.22.

This is okay to merge for RC, but I've not yet done a couple of things we should do for the final version:

don't warn when multi_class='warn' but y is binary
don't explicitly specify multi_class in docs/examples/tests when warning will therefore be silenced.

I may not have time to finish this for a couple of days.

Supersedes and closes #11476
Supersedes and closes #10001
Closes #9997

This reverts commit d06bad1.

rth

LGTM. I think the multi_class= 'auto' is indeed better than the earlier alternatives.

+1 for merging and fixing the remaining points mentioned in the description after the RC.

@amueller or @agramfort would you be able to have a look?

rth · 2018-08-24T11:47:02Z

doc/whats_new/v0.20.rst

+  :class:`linear_model.LogisticRegression` will change respectively from
+  ``'liblinear'`` and ``'ovr'`` in version 0.20 to ``'lbfgs'`` and
+  ``'auto'`` in version 0.22. A FutureWarning is raised when the default
+  values are used. :issue:`11476` by `Tom Dupre la Tour`_.


Maybe add yourself

rth · 2018-08-24T11:49:20Z

sklearn/linear_model/logistic.py

            Y_multi = le.fit_transform(y).astype(X.dtype, copy=False)

-        w0 = np.zeros((classes.size, n_features + int(fit_intercept)),
+        w0 = np.zeros((max(2, classes.size), n_features + int(fit_intercept)),


@amueller 's #11476 (comment) still apply here I think

It seems this can be safely reverted. I think we block classes.size < 2...

rth · 2018-08-24T11:51:28Z

sklearn/linear_model/logistic.py

+
+    if multi_class == 'auto':
+        multi_class = ('ovr' if coefs[0].shape[0] <= 2 or solver == 'liblinear'
+                       else 'multinomial')


Could we use _check_multi_class here instead of the above line, or is that redundant?

And this can be safely removed. _log_reg_scoring_path assumes pre-validation.

jnothman · 2018-08-25T10:38:59Z

If you've verified that you're happy with the new tests, I think we can just merge and get the release on its way.

jnothman · 2018-08-25T10:46:54Z

btw, I did change it to not warn upon binary, but I've not fixed up the examples, docs, tests to not pass redundantly.

jnothman · 2018-08-25T10:48:20Z

Oh, this didn't change any examples, so...

jnothman

Ready for merge.

rth · 2018-08-26T19:50:09Z

sklearn/linear_model/tests/test_logistic.py

+    y_bin = y_multi == 0
+    est_auto_bin = fit(X, y_bin, multi_class='auto', solver=solver)
+    est_ovr_bin = fit(X, y_bin, multi_class='ovr', solver=solver)
+    assert np.allclose(est_auto_bin.coef_, est_ovr_bin.coef_)


Not critical, but I think assert_allclose produces better trackbacks in the case of failure, also it has a mores strict default relative and absolute tolerances we might want to use one or the other but not mix both.

rth · 2018-08-26T19:54:43Z

sklearn/linear_model/tests/test_logistic.py

+                                   solver=solver).coef_)
+        assert not np.allclose(est_auto_bin.coef_,
+                               fit(X, y_multi, multi_class='multinomial',
+                                   solver=solver).coef_)


Overall I think these tests are fairly thorough!

rth · 2018-08-26T19:58:59Z

Merging with a +2 in the parent PR (#11476) and another +1 from #11476 (comment) for the added changes.

Thanks a lot @jnothman for all your work on making the RC happen!

* tag '0.20rc1': (1109 commits) MNT rc version DOC Release dates for 0.20 (scikit-learn#11838) DOC Fix: require n_splits > 1 in TimeSeriesSplit (scikit-learn#11937) FIX xfail for MacOS LogisticRegressionCV stability (scikit-learn#11936) MNT: Use GEMV in enet_coordinate_descent (Pt. 1) (scikit-learn#11896) [MRG] TST/FIX stop optics reachability failure on 32bit (scikit-learn#11916) ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' (scikit-learn#11905) MAINT Fix test_logistic::test_dtype_match failure on 32 bit arch (scikit-learn#11899) DOC Updated link to Laurens van der Maaten's home page (scikit-learn#11907) DOC Remove stray backtick in /doc/modules/feature_extraction.rst (scikit-learn#11910) Deprecate min_samples_leaf and min_weight_fraction_leaf (scikit-learn#11870) MNT modify test_sparse_oneclasssvm to be parametrized (scikit-learn#11894) EXA set figure size to avoid overlaps (scikit-learn#11889) MRG/REL fixes /skips for 32bit tests (scikit-learn#11879) add durations=20 to makefile to show test runtimes locally (scikit-learn#11147) DOC loss='l2' is no longer accpeted in l1_min_c DOC add note about brute force nearest neighbors for string data (scikit-learn#11884) DOC Change sign of energy in RBM (scikit-learn#11156) RFC try to warn on iid less often (scikit-learn#11613) DOC reduce plot_gpr_prior_posterior.py warnings(scikit-learn#11664) ...

* releases: (1109 commits) MNT rc version DOC Release dates for 0.20 (scikit-learn#11838) DOC Fix: require n_splits > 1 in TimeSeriesSplit (scikit-learn#11937) FIX xfail for MacOS LogisticRegressionCV stability (scikit-learn#11936) MNT: Use GEMV in enet_coordinate_descent (Pt. 1) (scikit-learn#11896) [MRG] TST/FIX stop optics reachability failure on 32bit (scikit-learn#11916) ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' (scikit-learn#11905) MAINT Fix test_logistic::test_dtype_match failure on 32 bit arch (scikit-learn#11899) DOC Updated link to Laurens van der Maaten's home page (scikit-learn#11907) DOC Remove stray backtick in /doc/modules/feature_extraction.rst (scikit-learn#11910) Deprecate min_samples_leaf and min_weight_fraction_leaf (scikit-learn#11870) MNT modify test_sparse_oneclasssvm to be parametrized (scikit-learn#11894) EXA set figure size to avoid overlaps (scikit-learn#11889) MRG/REL fixes /skips for 32bit tests (scikit-learn#11879) add durations=20 to makefile to show test runtimes locally (scikit-learn#11147) DOC loss='l2' is no longer accpeted in l1_min_c DOC add note about brute force nearest neighbors for string data (scikit-learn#11884) DOC Change sign of energy in RBM (scikit-learn#11156) RFC try to warn on iid less often (scikit-learn#11613) DOC reduce plot_gpr_prior_posterior.py warnings(scikit-learn#11664) ...

* dfsg: (1109 commits) MNT rc version DOC Release dates for 0.20 (scikit-learn#11838) DOC Fix: require n_splits > 1 in TimeSeriesSplit (scikit-learn#11937) FIX xfail for MacOS LogisticRegressionCV stability (scikit-learn#11936) MNT: Use GEMV in enet_coordinate_descent (Pt. 1) (scikit-learn#11896) [MRG] TST/FIX stop optics reachability failure on 32bit (scikit-learn#11916) ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' (scikit-learn#11905) MAINT Fix test_logistic::test_dtype_match failure on 32 bit arch (scikit-learn#11899) DOC Updated link to Laurens van der Maaten's home page (scikit-learn#11907) DOC Remove stray backtick in /doc/modules/feature_extraction.rst (scikit-learn#11910) Deprecate min_samples_leaf and min_weight_fraction_leaf (scikit-learn#11870) MNT modify test_sparse_oneclasssvm to be parametrized (scikit-learn#11894) EXA set figure size to avoid overlaps (scikit-learn#11889) MRG/REL fixes /skips for 32bit tests (scikit-learn#11879) add durations=20 to makefile to show test runtimes locally (scikit-learn#11147) DOC loss='l2' is no longer accpeted in l1_min_c DOC add note about brute force nearest neighbors for string data (scikit-learn#11884) DOC Change sign of energy in RBM (scikit-learn#11156) RFC try to warn on iid less often (scikit-learn#11613) DOC reduce plot_gpr_prior_posterior.py warnings(scikit-learn#11664) ...

TomDLT and others added 22 commits July 13, 2018 10:54

Change default solver in LogisticRegression

8f708da

simpler auto solver

83a5dec

Merge branch 'master' into logistic_default

25f343d

fix tests

679224b

fix more tests

a9219e8

fix doctests

284ecaa

remove 'auto' solver

cc53212

add whats_new entry

57b8364

fix doctests

1e21983

cleaning

85f0ecc

only warn for multi_class default if the problem is not binary

d06bad1

Merge branch 'master' into logistic_default

636e1c0

minor change

94e7927

FIX doctests

7342030

FIX doctest...

0ccf0b9

Revert "only warn for multi_class default if the problem is not binary"

115d34d

This reverts commit d06bad1.

This is an API change, not a feature

91be8bf

Merge branch 'master' into logistic_default

f7cd2fe

Decrease numerical precision in LogisticRegression doctest

bf3a7fd

More review comments

ec86d3b

ENH add multi_class='auto' for LR, default from 0.22

0896cc3

Fix some docs

b388487

jnothman changed the title ~~ENH add multi_class='auto' for LogisticRegression, default from 0.22~~ ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' Aug 24, 2018

jnothman added 2 commits August 24, 2018 13:20

What's new

dab350a

No warning when binary

ee1435d

rth approved these changes Aug 24, 2018

View reviewed changes

Respond to Roman

edd1e0d

jnothman commented Aug 25, 2018

View reviewed changes

jnothman changed the title ~~ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs'~~ [MRG+1] ENH add multi_class='auto' for LogisticRegression, default from 0.22; default solver will be 'lbfgs' Aug 25, 2018

rth reviewed Aug 26, 2018

View reviewed changes

rth merged commit 7ed61a2 into scikit-learn:master Aug 26, 2018

ogrisel mentioned this pull request Sep 19, 2018

[MRG] Fix FutureWarnings in logistic regression examples #12114

Merged

oleksandr-pavlyk mentioned this pull request Sep 28, 2018

test_logistic_regression_multi_class_auto contains unwarranted asserts #12193

Closed

Uh oh!

Conversation

jnothman commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

rth Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

rth Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

jnothman Aug 25, 2018

Choose a reason for hiding this comment

Uh oh!

rth Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

jnothman Aug 25, 2018

Choose a reason for hiding this comment

Uh oh!

jnothman commented Aug 25, 2018

Uh oh!

jnothman commented Aug 25, 2018

Uh oh!

jnothman commented Aug 25, 2018

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

rth Aug 26, 2018

Choose a reason for hiding this comment

Uh oh!

rth Aug 26, 2018

Choose a reason for hiding this comment

Uh oh!

rth commented Aug 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jnothman commented Aug 24, 2018 •

edited

Loading