FIX unintentional sample_weight upcast in CalibratedClassifierCV (scikit-learn#30873)

Author: ogrisel

doc/whats_new/upcoming_changes/sklearn.calibration/30873.fix.rst

@@ -0,0 +1,7 @@
+- :class:`~calibration.CalibratedClassifierCV` now raises `FutureWarning`
+  instead of `UserWarning` when passing `cv="prefit`". By
+  :user:`Olivier Grisel <ogrisel>`
+- :class:`~calibration.CalibratedClassifierCV` with `method="sigmoid"` no
+  longer crashes when passing `float64`-dtyped `sample_weight` along with a
+  base estimator that outputs `float32`-dtyped predictions. By :user:`Olivier
+  Grisel <ogrisel>`

sklearn/calibration.py

@@ -318,9 +318,6 @@ def fit(self, X, y, sample_weight=None, **fit_params):
         """
         check_classification_targets(y)
         X, y = indexable(X, y)
-        if sample_weight is not None:
-            sample_weight = _check_sample_weight(sample_weight, X)
-
         estimator = self._get_estimator()
 
         _ensemble = self.ensemble
@@ -333,7 +330,8 @@ def fit(self, X, y, sample_weight=None, **fit_params):
             warnings.warn(
                 "The `cv='prefit'` option is deprecated in 1.6 and will be removed in"
                 " 1.8. You can use CalibratedClassifierCV(FrozenEstimator(estimator))"
-                " instead."
+                " instead.",
+                category=FutureWarning,
             )
             # `classes_` should be consistent with that of estimator
             check_is_fitted(self.estimator, attributes=["classes_"])
@@ -348,6 +346,13 @@ def fit(self, X, y, sample_weight=None, **fit_params):
                 # Reshape binary output from `(n_samples,)` to `(n_samples, 1)`
                 predictions = predictions.reshape(-1, 1)
 
+            if sample_weight is not None:
+                # Check that the sample_weight dtype is consistent with the predictions
+                # to avoid unintentional upcasts.
+                sample_weight = _check_sample_weight(
+                    sample_weight, predictions, dtype=predictions.dtype
+                )
+
             calibrated_classifier = _fit_calibrator(
                 estimator,
                 predictions,
@@ -457,6 +462,13 @@ def fit(self, X, y, sample_weight=None, **fit_params):
                         )
                     predictions = predictions.reshape(-1, 1)
 
+                if sample_weight is not None:
+                    # Check that the sample_weight dtype is consistent with the
+                    # predictions to avoid unintentional upcasts.
+                    sample_weight = _check_sample_weight(
+                        sample_weight, predictions, dtype=predictions.dtype
+                    )
+
                 this_estimator.fit(X, y, **routed_params.estimator.fit)
                 # Note: Here we don't pass on fit_params because the supported
                 # calibrators don't support fit_params anyway
@@ -622,7 +634,13 @@ def _fit_classifier_calibrator_pair(
         # Reshape binary output from `(n_samples,)` to `(n_samples, 1)`
         predictions = predictions.reshape(-1, 1)
 
-    sw_test = None if sample_weight is None else _safe_indexing(sample_weight, test)
+    if sample_weight is not None:
+        # Check that the sample_weight dtype is consistent with the predictions
+        # to avoid unintentional upcasts.
+        sample_weight = _check_sample_weight(sample_weight, X, dtype=predictions.dtype)
+        sw_test = _safe_indexing(sample_weight, test)
+    else:
+        sw_test = None
     calibrated_classifier = _fit_calibrator(
         estimator, predictions, y_test, classes, method, sample_weight=sw_test
     )

sklearn/tests/test_calibration.py

@@ -579,8 +579,12 @@ def test_calibration_attributes(clf, cv):
     X, y = make_classification(n_samples=10, n_features=5, n_classes=2, random_state=7)
     if cv == "prefit":
         clf = clf.fit(X, y)
-    calib_clf = CalibratedClassifierCV(clf, cv=cv)
-    calib_clf.fit(X, y)
+        calib_clf = CalibratedClassifierCV(clf, cv=cv)
+        with pytest.warns(FutureWarning):
+            calib_clf.fit(X, y)
+    else:
+        calib_clf = CalibratedClassifierCV(clf, cv=cv)
+        calib_clf.fit(X, y)
 
     if cv == "prefit":
         assert_array_equal(calib_clf.classes_, clf.classes_)
@@ -1077,20 +1081,48 @@ def test_sigmoid_calibration_max_abs_prediction_threshold(global_random_seed):
     assert_allclose(b2, b3, atol=atol)
 
 
-def test_float32_predict_proba(data):
+@pytest.mark.parametrize("use_sample_weight", [True, False])
+@pytest.mark.parametrize("method", ["sigmoid", "isotonic"])
+def test_float32_predict_proba(data, use_sample_weight, method):
     """Check that CalibratedClassifierCV works with float32 predict proba.
 
-    Non-regression test for gh-28245.
+    Non-regression test for gh-28245 and gh-28247.
     """
+    if use_sample_weight:
+        # Use dtype=np.float64 to check that this does not trigger an
+        # unintentional upcasting: the dtype of the base estimator should
+        # control the dtype of the final model. In particular, the
+        # sigmoid calibrator relies on inputs (predictions and sample weights)
+        # with consistent dtypes because it is partially written in Cython.
+        # As this test forces the predictions to be `float32`, we want to check
+        # that `CalibratedClassifierCV` internally converts `sample_weight` to
+        # the same dtype to avoid crashing the Cython call.
+        sample_weight = np.ones_like(data[1], dtype=np.float64)
+    else:
+        sample_weight = None
 
     class DummyClassifer32(DummyClassifier):
         def predict_proba(self, X):
             return super().predict_proba(X).astype(np.float32)
 
     model = DummyClassifer32()
-    calibrator = CalibratedClassifierCV(model)
-    # Does not raise an error
-    calibrator.fit(*data)
+    calibrator = CalibratedClassifierCV(model, method=method)
+    # Does not raise an error.
+    calibrator.fit(*data, sample_weight=sample_weight)
+
+    # Check with frozen prefit model
+    model = DummyClassifer32().fit(*data, sample_weight=sample_weight)
+    calibrator = CalibratedClassifierCV(FrozenEstimator(model), method=method)
+    # Does not raise an error.
+    calibrator.fit(*data, sample_weight=sample_weight)
+
+    # TODO(1.8): remove me once the deprecation period is over.
+    # Check with prefit model using the deprecated cv="prefit" argument:
+    model = DummyClassifer32().fit(*data, sample_weight=sample_weight)
+    calibrator = CalibratedClassifierCV(model, method=method, cv="prefit")
+    # Does not raise an error.
+    with pytest.warns(FutureWarning):
+        calibrator.fit(*data, sample_weight=sample_weight)
 
 
 def test_error_less_class_samples_than_folds():