FIX improve error message for large sparse matrix input in LogisticRegression (#21093)

TomDLT · glemaitre · commit 8aebfe59c5c0 · 2021-10-25T11:37:00.000+02:00
diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst
@@ -0,0 +1,76 @@
+.. include:: _contributors.rst
+
+.. currentmodule:: sklearn
+
+.. _changes_1_1:
+
+Version 1.1.0
+=============
+
+**In Development**
+
+
+.. include:: changelog_legend.inc
+
+Minimal dependencies
+--------------------
+
+Version 1.1.0 of scikit-learn requires python 3.7+, numpy 1.14.6+ and
+scipy 1.1.0+. Optional minimal dependency is matplotlib 2.2.2+.
+
+Put the changes in their relevant module.
+
+Changed models
+--------------
+
+
+Changelog
+---------
+
+..
+    Entries should be grouped by module (in alphabetic order) and prefixed with
+    one of the labels: |MajorFeature|, |Feature|, |Efficiency|, |Enhancement|,
+    |Fix| or |API| (see whats_new.rst for descriptions).
+    Entries should be ordered by those labels (e.g. |Fix| after |Efficiency|).
+    Changes not specific to a module should be listed under *Multiple Modules*
+    or *Miscellaneous*.
+    Entries should end with:
+    :pr:`123456` by :user:`Joe Bloggs <joeongithub>`.
+    where 123456 is the *pull request* number, not the issue number.
+
+:mod:`sklearn.linear_model`
+...........................
+
+- |Fix| :class:`linear_model.LogisticRegression` now raises a better error
+  message when the solver does not support sparse matrices with int64 indices.
+  :pr:`21093` by `Tom Dupre la Tour`_.
+
+:mod:`sklearn.utils`
+....................
+
+- |Enhancement| :func:`utils.validation._check_sample_weight` can perform a
+  non-negativity check on the sample weights. It can be turned on
+  using the only_non_negative bool parameter.
+  Estimators that check for non-negative weights are updated:
+  :func:`linear_model.LinearRegression` (here the previous
+  error message was misleading),
+  :func:`ensemble.AdaBoostClassifier`,
+  :func:`ensemble.AdaBoostRegressor`,
+  :func:`neighbors.KernelDensity`.
+  :pr:`20880` by :user:`Guillaume Lemaitre <glemaitre>`
+  and :user:`András Simon <simonandras>`.
+
+:mod:`sklearn.pipeline`
+.......................
+
+- |Enhancement| Added support for "passthrough" in :class:`FeatureUnion`.
+  Setting a transformer to "passthrough" will pass the features unchanged.
+  :pr:`20860` by :user:`Shubhraneel Pal <shubhraneel>`.
+
+Code and Documentation Contributors
+-----------------------------------
+
+Thanks to everyone who has contributed to the maintenance and improvement of
+the project since version 1.0, including:
+
+TODO: update at the time of the release.
diff --git a/sklearn/linear_model/_logistic.py b/sklearn/linear_model/_logistic.py
@@ -663,7 +663,7 @@ def _logistic_regression_path(
             X,
             accept_sparse="csr",
             dtype=np.float64,
-            accept_large_sparse=solver != "liblinear",
+            accept_large_sparse=solver not in ["liblinear", "sag", "saga"],
         )
         y = check_array(y, ensure_2d=False, dtype=None)
         check_consistent_length(X, y)
@@ -1511,7 +1511,7 @@ def fit(self, X, y, sample_weight=None):
             accept_sparse="csr",
             dtype=_dtype,
             order="C",
-            accept_large_sparse=solver != "liblinear",
+            accept_large_sparse=solver not in ["liblinear", "sag", "saga"],
         )
         check_classification_targets(y)
         self.classes_ = np.unique(y)
@@ -2080,7 +2080,7 @@ def fit(self, X, y, sample_weight=None):
             accept_sparse="csr",
             dtype=np.float64,
             order="C",
-            accept_large_sparse=solver != "liblinear",
+            accept_large_sparse=solver not in ["liblinear", "sag", "saga"],
         )
         check_classification_targets(y)
 
diff --git a/sklearn/linear_model/tests/test_logistic.py b/sklearn/linear_model/tests/test_logistic.py
@@ -2237,3 +2237,22 @@ def test_sample_weight_not_modified(multi_class, class_weight):
     )
     clf.fit(X, y, sample_weight=W)
     assert_allclose(expected, W)
+
+
+@pytest.mark.parametrize("solver", ["liblinear", "lbfgs", "newton-cg", "sag", "saga"])
+def test_large_sparse_matrix(solver):
+    # Solvers either accept large sparse matrices, or raise helpful error.
+    # Non-regression test for pull-request #21093.
+
+    # generate sparse matrix with int64 indices
+    X = sp.rand(20, 10, format="csr")
+    for attr in ["indices", "indptr"]:
+        setattr(X, attr, getattr(X, attr).astype("int64"))
+    y = np.random.randint(2, size=X.shape[0])
+
+    if solver in ["liblinear", "sag", "saga"]:
+        msg = "Only sparse matrices with 32-bit integer indices"
+        with pytest.raises(ValueError, match=msg):
+            LogisticRegression(solver=solver).fit(X, y)
+    else:
+        LogisticRegression(solver=solver).fit(X, y)
