Repository: scikit-learn/scikit-learn
Pull request: Fix buffer dtype mismatch in isotonic regression
Fixes https://github.com/scikit-learn/scikit-learn/issues/15004

Error replay:
```
from sklearn import isotonic
import numpy as np
m = isotonic.IsotonicRegression()
m.fit(np.zeros((10,),dtype='float32'), np.zeros((10,),dtype='int64'))
```
Gives
```
File "sklearn/_isotonic.pyx", line 66, in sklearn._isotonic._make_unique
ValueError: Buffer dtype mismatch, expected 'float' but got 'double'
```
Tested under : `scikit-learn==0.21.3` `numpy==0.17.0`.

A more realistic scenario is creating an XGBClassifier model (with `xgboost==0.90`), and run `sklearn.calibration.CalibratedClassifierCV` on it. The same error happens.

I am not good at Cython but I think the reason is by using `check_array`, `X` and `y` get converted to different types. Here is a simple fix that make the code work, I think there should be a better way to fix it in Cython code.

Problem statement:
Numpy float32 giving buffer mismatch error (as double) in Cython code of Isotonic regression
#### Description
The `_make_unique` method in `_isotonic.pyx` throws buffer mismatch error (only when using float 32, which seems to be recast as double). The error doesn't come up when using float64

(Larger story: I am using CalibratedClassifierCV with isotonic calibration with the probabilities produced by an XGBoost model (which are in float32). CalibratedClassifierCV does not check for float64 before shipping off the probabilities to IsotonicRegression, which is what started the problem).
 
#### Steps/Code to Reproduce
Minimal example:
```
a = np.random.normal(size = 100)                                     
b = np.random.choice([0, 1], p = [0.5, 0.5], size = 100)                         
from sklearn.isotonic import IsotonicRegression                      
regress = IsotonicRegression()
regress.fit(a.astype('float32'), b)
```
The problem does not arise if a is in its default type of float64.

#### Expected Results
No error is thrown, as happens when a is cast as float64.

#### Actual Results
```
~/anaconda3/envs/xgboost_test/lib/python3.7/site-packages/sklearn/isotonic.py in fit(self, X, y, sample_weight)
    333         # Transform y by running the isotonic regression algorithm and
    334         # transform X accordingly.
--> 335         X, y = self._build_y(X, y, sample_weight)
    336 
    337         # It is necessary to store the non-redundant part of the training set

~/anaconda3/envs/xgboost_test/lib/python3.7/site-packages/sklearn/isotonic.py in _build_y(self, X, y, sample_weight, trim_duplicates)
    271         X, y, sample_weight = [array[order] for array in [X, y, sample_weight]]
    272         unique_X, unique_y, unique_sample_weight = _make_unique(
--> 273             X, y, sample_weight)
    274 
    275         # Store _X_ and _y_ to maintain backward compat during the deprecation

sklearn/_isotonic.pyx in sklearn._isotonic._make_unique()

ValueError: Buffer dtype mismatch, expected 'float' but got 'double'
```
#### Versions
```
In [114]: sklearn.show_versions()                                              

System:
    python: 3.7.4 (default, Aug 13 2019, 15:17:50)  [Clang 4.0.1 (tags/RELEASE_401/final)]
executable: /Users/nmukherjee/anaconda3/envs/xgboost_test/bin/python
   machine: Darwin-18.7.0-x86_64-i386-64bit

BLAS:
    macros: SCIPY_MKL_H=None, HAVE_CBLAS=None
  lib_dirs: /Users/nmukherjee/anaconda3/envs/xgboost_test/lib
cblas_libs: mkl_rt, pthread

Python deps:
       pip: 19.2.2
setuptools: 41.0.1
   sklearn: 0.21.2
     numpy: 1.16.5
     scipy: 1.3.1
    Cython: None
    pandas: 0.25.1
```
#### Dealing with the problem
Either figuring out how to deal with float32 within IsotonicRegression, or checking (and recasting to) float64 within CalibratedClassifierCV

Hints from issue discussion:

Commit message: fix mismatched dtype problem. added test.

Patch to review:
diff --git a/sklearn/isotonic.py b/sklearn/isotonic.py
index 6e473334ad1c8..aa70b15cff769 100644
--- a/sklearn/isotonic.py
+++ b/sklearn/isotonic.py
@@ -324,10 +324,9 @@ def fit(self, X, y, sample_weight=None):
         X is stored for future use, as :meth:`transform` needs X to interpolate
         new input data.
         """
-        check_params = dict(accept_sparse=False, ensure_2d=False,
-                            dtype=[np.float64, np.float32])
-        X = check_array(X, **check_params)
-        y = check_array(y, **check_params)
+        check_params = dict(accept_sparse=False, ensure_2d=False)
+        X = check_array(X, dtype=[np.float64, np.float32], **check_params)
+        y = check_array(y, dtype=X.dtype, **check_params)
         check_consistent_length(X, y, sample_weight)
 
         # Transform y by running the isotonic regression algorithm and
diff --git a/sklearn/tests/test_isotonic.py b/sklearn/tests/test_isotonic.py
index 5f22a89ba23a1..fbaf13a620a92 100644
--- a/sklearn/tests/test_isotonic.py
+++ b/sklearn/tests/test_isotonic.py
@@ -385,19 +385,19 @@ def test_isotonic_ymin_ymax():
                   -0.896, -0.377, -1.327, 0.180])
     y = isotonic_regression(x, y_min=0., y_max=0.1)
 
-    assert(np.all(y >= 0))
-    assert(np.all(y <= 0.1))
+    assert (np.all(y >= 0))
+    assert (np.all(y <= 0.1))
 
     # Also test decreasing case since the logic there is different
     y = isotonic_regression(x, y_min=0., y_max=0.1, increasing=False)
 
-    assert(np.all(y >= 0))
-    assert(np.all(y <= 0.1))
+    assert (np.all(y >= 0))
+    assert (np.all(y <= 0.1))
 
     # Finally, test with only one bound
     y = isotonic_regression(x, y_min=0., increasing=False)
 
-    assert(np.all(y >= 0))
+    assert (np.all(y >= 0))
 
 
 def test_isotonic_zero_weight_loop():
@@ -485,6 +485,16 @@ def test_isotonic_dtype():
             assert res.dtype == expected_dtype
 
 
+def test_isotonic_mismatched_dtype():
+    reg = IsotonicRegression()
+    y = [2, 1, 4, 3, 5]
+    for dtype in (np.int32, np.int64, np.float64):
+        y_np = np.array(y, dtype=dtype)
+        X = np.arange(len(y)).astype(np.float32)
+        reg.fit(X, y_np)
+        assert reg.predict(X).dtype == X.dtype
+
+
 def test_make_unique_dtype():
     x_list = [2, 2, 2, 3, 5]
     for dtype in (np.float32, np.float64):


Code review comments:
diff_hunk: @@ -385,19 +385,19 @@ def test_isotonic_ymin_ymax():
                   -0.896, -0.377, -1.327, 0.180])
     y = isotonic_regression(x, y_min=0., y_max=0.1)
 
-    assert(np.all(y >= 0))
-    assert(np.all(y <= 0.1))
+    assert (np.all(y >= 0))
path: sklearn/tests/test_isotonic.py
text: @user1:
if you're going to do a cosmetic clean-up here, it would be best to remove the parentheses.

diff_hunk: @@ -485,6 +485,16 @@ def test_isotonic_dtype():
             assert res.dtype == expected_dtype
 
 
+def test_isotonic_mismatched_dtype():
+    reg = IsotonicRegression()
+    y = [2, 1, 4, 3, 5]
+    for dtype in (np.int32, np.int64, np.float64):
+        y_np = np.array(y, dtype=dtype)
+        X = np.arange(len(y)).astype(np.float32)
path: sklearn/tests/test_isotonic.py
text: @user1:
Nit: Better to make `X`  2D instead of relying on `check_array` to convert it,

```suggestion
        X = np.arange(len(y), dtype=np.float32).reshape((-1, 1))
```

@author:
I am not very sure about reshaping here because I wonder if the not-reshaped input is intended as part of the test. So I kept it as 1D like previous.

@user1:
Exactly, the current version make one wonder whether you want to also check that the input is reshaped to 2D. However that's not the intention of this test as far as I can tell? Therefore better to do one thing, and only check dtype.

@author:
I see, I merged your suggestions into the test. Thanks.