[MRG+1] ENH Ridge with solver SAG/SAGA does not cast to float64 by massich · Pull Request #13302 · scikit-learn/scikit-learn

massich · 2019-02-27T10:28:25Z

closes #11642, (also closes #11643 by taking over)

build upon #11155

TODO:

~~Merge [MRG] LogisticRegression convert to float64 (sag) #11155 to reduce the diff.~~ rebase master when LogisticRegression convert to float64 (for SAG solver) #13243 gets in
Ensure that the casting rule is clear between base classes, classes and functions. I suspect that we have some copy which are not useful.

massich · 2019-02-27T12:30:28Z

I get a segfall that I don't manage to reproduce

massich · 2019-02-28T10:36:19Z

sklearn/linear_model/tests/test_ridge.py

+    assert results[np.float32].dtype == np.float32
+    assert results[np.float64].dtype == np.float64
+    assert_allclose(results[np.float32], results[np.float64],
+                    rtol=assert_tolerance)


The types are maintained but the numerical results are not stable. See:

~/code/scikit-learn is/ridge_regression_sag* (mne) ❯ pytest sklearn/linear_model/tests/test_ridge.py -k dtype_stability -v --tb=no Test session starts (platform: linux, Python 3.6.6, pytest 4.0.0, pytest-sugar 0.9.2) cachedir: .pytest_cache rootdir: /home/sik/code/scikit-learn, inifile: setup.cfg plugins: sugar-0.9.2, pudb-0.7.0, mock-1.10.0, faulthandler-1.5.0, cov-2.6.0 collecting ... sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-svd] ✓ 4% ▌ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-cholesky] ✓ 8% ▉ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-lsqr] ✓ 12% █▍ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-sparse_cg] ✓ 17% █▋ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-sag] ✓ 21% ██▏ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.1-saga] ✓ 25% ██▌ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-svd] ✓ 29% ██▉ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-cholesky] ✓ 33% ███▍ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-lsqr] ✓ 38% ███▊ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-sparse_cg] ✓ 42% ████▎ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-sag] ⨯ 46% ████▋ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.01-saga] ✓ 50% █████ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-svd] ✓ 54% █████▌ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-cholesky] ✓ 58% █████▉ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-lsqr] ✓ 62% ██████▍ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-sparse_cg] ✓ 67% ██████▋ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-sag] ⨯ 71% ███████▏ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[0.001-saga] ⨯ 75% ███████▌ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-svd] ✓ 79% ███████▉ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-cholesky] ✓ 83% ████████▍ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-lsqr] ⨯ 88% ████████▊ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-sparse_cg] ✓ 92% █████████▎ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-sag] ⨯ 96% █████████▋ sklearn/linear_model/tests/test_ridge.py::test_ridge_regression_dtype_stability[1e-06-saga] ⨯ 100% ██████████ Results (0.66s): 18 passed 6 failed - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[0.01-sag] - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[0.001-sag] - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[0.001-saga] - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[1e-06-lsqr] - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[1e-06-sag] - sklearn/linear_model/tests/test_ridge.py:943 test_ridge_regression_dtype_stability[1e-06-saga] 49 deselected

Similar issues can be found #13303 and #13309

what is weird compared to #13309 is that svd is stable

GaelVaroquaux · 2019-02-28T13:58:11Z

There are segfaults in the tests on azure :(

massich · 2019-02-28T15:15:14Z

There are segfaults in the tests on azure :(

@GaelVaroquaux :) yes! The thing is that sag only supports csr and we are skipping the check, so it segfaults. If none of the other solvers supports the other things, then just hardcoding csr is enough, otherwise we need to move the validation logic ouside.

massich · 2019-02-28T16:59:33Z

I was trying to see which solver prefers which sparse configuration in our doc, and I could not find it:
https://scikit-learn.org/stable/modules/generated/sklearn.linear_model.Ridge.html#sklearn.linear_model.Ridge.fit
https://scikit-learn.org/stable/modules/linear_model.html#ridge-regression
https://scikit-learn.org/stable/modules/generated/sklearn.linear_model.ridge_regression.html#sklearn.linear_model.ridge_regression

on the other side, when we use things like scipy's lsqr I didn't figure out which sparse preference the algorithm has. I guess that clue is the first thing that lsqr does is to call this
https://github.com/scipy/scipy/blob/f2ec91c4908f9d67b5445fbfacce7f47518b35d1/scipy/sparse/linalg/interface.py#L381:5

agramfort

LGTM.

+1 for MRG if CIs are green

please also update what's new

sklearn/linear_model/tests/test_ridge.py

massich · 2019-03-01T09:24:34Z

cross reference #11000

GaelVaroquaux · 2019-03-01T16:28:49Z

Test failures are legit and easy to fix: it's "RandomState", and not "randomstate"

sklearn/linear_model/tests/test_ridge.py

GaelVaroquaux

LGTM aside from the randomstate issues in the test.

jnothman · 2019-05-01T07:02:19Z

This seems to have approvals, but just needs finishing touches. Assuming we don't decide to squeeze it into 0.21 this week, what's new entry should be moved to 0.22.

glemaitre · 2019-05-03T15:43:52Z

@jnothman @GaelVaroquaux I corrected the tests. Do you want to merge for 0.21 or 0.22.
If 0.22, I'll change the what's new.

jnothman · 2019-05-04T23:15:18Z

I think it should be fine to release, but we will need to start pulling together the list of issues for final release, and choose a date for release.

…#13302)

massich changed the title ~~Is/ridge regression sag~~ EHN: Ridge with solver SAG/SAGA does not cast to float64 Feb 27, 2019

massich force-pushed the is/ridge_regression_sag branch from 872ebf5 to 5630c25 Compare February 27, 2019 10:33

iter

c95f6ba

massich force-pushed the is/ridge_regression_sag branch from 5630c25 to c95f6ba Compare February 27, 2019 10:44

use allclose

132cb33

Test the ridge_regression inner function

8ea1716

massich commented Feb 28, 2019

View reviewed changes

Fix tests

b546b45

massich marked this pull request as ready for review February 28, 2019 11:16

Joan Massich added 2 commits February 28, 2019 15:18

TST: replicate Segfault

482c837

fix: force to use csr

bd57ff8

Merge branch 'master' into is/ridge_regression_sag

1e9881f

tweak windows tolerance

e3df1df

massich force-pushed the is/ridge_regression_sag branch from 98c4eda to e3df1df Compare February 28, 2019 17:44

Joan Massich added 4 commits February 28, 2019 18:48

fix tolerance

b71046a

Add something to choose preferred sparcity based on solver

cb37eec

Remove redundant test

b65ed10

clean up

ba91cc6

agramfort changed the title ~~EHN: Ridge with solver SAG/SAGA does not cast to float64~~ ENH: Ridge with solver SAG/SAGA does not cast to float64 Mar 1, 2019

agramfort reviewed Mar 1, 2019

View reviewed changes

sklearn/linear_model/tests/test_ridge.py Outdated Show resolved Hide resolved

Joan Massich added 4 commits March 1, 2019 09:47

remove list comprehension

9a910c8

remove caprials

760a2e5

update whatsnew

6cc71c1

Merge branch 'master' into is/ridge_regression_sag

7f8ff9d

glemaitre mentioned this pull request Mar 1, 2019

Preserving dtype for float32 / float64 in transformers #11000

Open

28 tasks

GaelVaroquaux reviewed Mar 1, 2019

View reviewed changes

sklearn/linear_model/tests/test_ridge.py Outdated Show resolved Hide resolved

GaelVaroquaux reviewed Mar 1, 2019

View reviewed changes

sklearn/linear_model/tests/test_ridge.py Outdated Show resolved Hide resolved

GaelVaroquaux reviewed Mar 1, 2019

View reviewed changes

glemaitre self-assigned this May 2, 2019

glemaitre added 2 commits May 3, 2019 16:40

Merge remote-tracking branch 'origin/master' into pr/massich/13302

886669b

TST solve issues

e196c8d

glemaitre changed the title ~~ENH: Ridge with solver SAG/SAGA does not cast to float64~~ [MRG] ENH Ridge with solver SAG/SAGA does not cast to float64 May 3, 2019

glemaitre changed the title ~~[MRG] ENH Ridge with solver SAG/SAGA does not cast to float64~~ [MRG+1] ENH Ridge with solver SAG/SAGA does not cast to float64 May 3, 2019

fix

496adb1

jnothman merged commit 0dac63f into scikit-learn:master May 4, 2019

jnothman pushed a commit to jnothman/scikit-learn that referenced this pull request May 6, 2019

ENH Ridge with solver SAG/SAGA does not cast to float64 (scikit-learn…

7d182ca

…#13302)

This was referenced May 7, 2019

Make test_ridge_regression_dtype_stability less random #13816

Merged

[MRG] handle sparse x and intercept in _RidgeGCV #13350

Merged

glemaitre mentioned this pull request May 8, 2019

MNT release 0.21.0 #13804

Merged

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

ENH Ridge with solver SAG/SAGA does not cast to float64 (scikit-learn…

fc11442

…#13302)

Uh oh!

Conversation

massich commented Feb 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

massich commented Feb 27, 2019

Uh oh!

massich Feb 28, 2019

Choose a reason for hiding this comment

Uh oh!

massich Feb 28, 2019

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux commented Feb 28, 2019

Uh oh!

massich commented Feb 28, 2019

Uh oh!

massich commented Feb 28, 2019

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

massich commented Mar 1, 2019

Uh oh!

GaelVaroquaux commented Mar 1, 2019

Uh oh!

Uh oh!

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman commented May 1, 2019

Uh oh!

glemaitre commented May 3, 2019

Uh oh!

jnothman commented May 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

massich commented Feb 27, 2019 •

edited

Loading