Repository: scikit-learn/scikit-learn
Pull request: [MRG] FIX TFIDFVectorizer follow parameter dtype and TFIDFTransformer preserve dtype
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs
Fixes #10411 #8044
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->


#### What does this implement/fix? Explain your changes.
- [x] TfidfVectorizer now respects data type
- [x] TfidfVectorizer validates intput and casts if int*
- [x] 2 test cases to check the changes

#### Any other comments?


<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
TfidfVectorizer dtype argument ignored
#### Description
TfidfVectorizer's fit/fit_transform output is always np.float64 instead of the specified dtype

#### Steps/Code to Reproduce
```py
from sklearn.feature_extraction.text import TfidfVectorizer
test = TfidfVectorizer(dtype=np.float32)
print(test.fit_transform(["Help I have a bug"]).dtype)
```

#### Expected Results
```py
dtype('float32')
```

#### Actual Results
```py
dtype('float64')
```

#### Versions
```
Darwin-17.2.0-x86_64-i386-64bit
Python 3.6.1 |Anaconda 4.4.0 (x86_64)| (default, May 11 2017, 13:04:09) 
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.57)]
NumPy 1.13.3
SciPy 1.0.0
Scikit-Learn 0.19.0
```

Hints from issue discussion:
Hi, I found the same problem as yours. The cause looks like that TfidfVectorizer uses TfidfTransformer underhood (as  _tfidf instance attribute). And when TfidfTransformer tries to fit the training data, it basically does nothing to adhere to what dtype argument specified. It just uses the system default dtype when converting from python int or float. The default dtype on  x64 platform is int64 and float64. Therefore,  the dtype is always promoted to float64 if no futher processing is done.      
      
I wonder if there's way to set default type to float32 on x64 platform?  Or maybe force dtype of idf_ and \_idf_diag to be consistent with input dtype?
Thanks for reporting this issue. Yes, it is related to https://github.com/scikit-learn/scikit-learn/issues/8044 and the fact that `TfidfTransformer` does not preserve dtype. A minimal code sample to reproduce could be,
```py
from sklearn.feature_extraction.text import TfidfTransformer
import scipy.sparse

X = scipy.sparse.random(10, 20000, dtype=np.float32, random_state=42)
X_idf = TfidfTransformer().fit_transform(X)
print(X.dtype, X_idf.dtype)
```
which yields `float32 float64`. 

A Pull Request to fix it would be wecome. I think, one could, add similar validation in `TfidfTranformer.fit` to the [one in `StandardScaler`](https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/preprocessing/data.py#L618).
  
Thanks for posting. I'd like to take it up

Commit message: Added separate test for vectorizer and transformer

Patch to review:
diff --git a/sklearn/feature_extraction/tests/test_text.py b/sklearn/feature_extraction/tests/test_text.py
index ff13cd6e00179..a42fe2bdd5b8b 100644
--- a/sklearn/feature_extraction/tests/test_text.py
+++ b/sklearn/feature_extraction/tests/test_text.py
@@ -1,6 +1,9 @@
 from __future__ import unicode_literals
 import warnings
 
+import pytest
+from scipy.sparse import rand
+
 from sklearn.feature_extraction.text import strip_tags
 from sklearn.feature_extraction.text import strip_accents_unicode
 from sklearn.feature_extraction.text import strip_accents_ascii
@@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64,
+])
+def test_tfidf_vectorizer_type(dtype):
+    X = np.array(["numpy", "scipy", "sklearn"])
+    vectorizer = TfidfVectorizer(dtype=dtype)
+    X_idf = vectorizer.fit_transform(X)
+    assert dtype == X_idf.dtype
+
+
+def test_tfidf_fallback():
+    # TFIDFTransformer should fallback to
+    # np.float64 if np.int* is passed
+    X = np.array([1, 2, 3, 4, 5], dtype=np.int32)
+    X_idf = TfidfTransformer().fit_transform(X)
+    assert X_idf.dtype.type == np.float64
diff --git a/sklearn/feature_extraction/text.py b/sklearn/feature_extraction/text.py
index a1e0845abe9ac..6277a0d414500 100644
--- a/sklearn/feature_extraction/text.py
+++ b/sklearn/feature_extraction/text.py
@@ -29,7 +29,7 @@
 from ..preprocessing import normalize
 from .hashing import FeatureHasher
 from .stop_words import ENGLISH_STOP_WORDS
-from ..utils.validation import check_is_fitted
+from ..utils.validation import check_is_fitted, FLOAT_DTYPES
 from ..utils.fixes import sp_version
 
 __all__ = ['CountVectorizer',
@@ -1068,17 +1068,21 @@ def fit(self, X, y=None):
         """
         if not sp.issparse(X):
             X = sp.csc_matrix(X)
+
         if self.use_idf:
             n_samples, n_features = X.shape
-            df = _document_frequency(X)
+            # _document_frequency will return int32
+            # array by default, hence the cast
+            df = _document_frequency(X).astype(X.dtype
+                                               if X.dtype in FLOAT_DTYPES
+                                               else np.float64)
 
             # perform idf smoothing if required
             df += int(self.smooth_idf)
             n_samples += int(self.smooth_idf)
-
             # log+1 instead of log makes sure terms with zero idf don't get
             # suppressed entirely.
-            idf = np.log(float(n_samples) / df) + 1.0
+            idf = np.log(n_samples / df) + 1.0
             self._idf_diag = sp.spdiags(idf, diags=0, m=n_features,
                                         n=n_features, format='csr')
 


Code review comments:
diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
remove this comment

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
remove this comment

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64,
+])
+def test_tfidf_vectorizer_type(dtype):
+    X = np.array(["numpy", "scipy", "sklearn"])
+    vectorizer = TfidfVectorizer(dtype=dtype)
+    X_idf = vectorizer.fit_transform(X)
+    assert dtype == X_idf.dtype
+
+
+def test_tfidf_fallback():
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
You don't need this test. It is a case of the first one. please add it there.

@user1:
So you probably need to create  X in the parametrize and to pass the dtype as well.

diff_hunk: @@ -1068,17 +1068,21 @@ def fit(self, X, y=None):
         """
         if not sp.issparse(X):
             X = sp.csc_matrix(X)
+
         if self.use_idf:
             n_samples, n_features = X.shape
-            df = _document_frequency(X)
+            # _document_frequency will return int32
+            # array by default, hence the cast
+            df = _document_frequency(X).astype(X.dtype
+                                               if X.dtype in FLOAT_DTYPES
+                                               else np.float64)
 
             # perform idf smoothing if required
             df += int(self.smooth_idf)
             n_samples += int(self.smooth_idf)
-
             # log+1 instead of log makes sure terms with zero idf don't get
             # suppressed entirely.
-            idf = np.log(float(n_samples) / df) + 1.0
+            idf = np.log(n_samples / df) + 1.0
path: sklearn/feature_extraction/text.py
text: @user1:
I would have thought we should always perform this log in float64, and only afterwards convert back to float32 if appropriate. Confused. I don't understand why you had that test failing before and not now.

@author:
Because `df` is converted to float if it's in int

@user1:
But shouldn't it be converted to float64 in all cases, then idf converted back to float32 if necessary?

@author:
Yeah it seems a little magical that the dtype after transforming matches even if I am not converting it back to the original type. I'll do a little debugging to answer this question.

@user1:
But you're also unnecessarily reducing the precision of the calculation here in the float32 case.

@author:
OK so the required behavior is always convert `df` to float64 for precision and convert `idf` back to X.dtype? 

@user2:
It was my proposal in https://github.com/glemaitre/scikit-learn/commit/4e5e1f06ed1b90c0ffb00584db81a4e8c77e1dff

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64,
+])
path: sklearn/feature_extraction/tests/test_text.py
text: @user2:
Try all combination which are possible:

```python
@user1.mark.parametrize(
    "vectorizer_dtype, output_dtype",
    [(np.int32, np.float64),
     (np.int64, np.float64),
     (np.float32, np.float32),
     (np.float64, np.float64)])
````

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64,
+])
+def test_tfidf_vectorizer_type(dtype):
+    X = np.array(["numpy", "scipy", "sklearn"])
+    vectorizer = TfidfVectorizer(dtype=dtype)
+    X_idf = vectorizer.fit_transform(X)
+    assert dtype == X_idf.dtype
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
inverse the checking

```python
assert X_idf.dtype = output_dtype
```

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64,
+])
+def test_tfidf_vectorizer_type(dtype):
+    X = np.array(["numpy", "scipy", "sklearn"])
+    vectorizer = TfidfVectorizer(dtype=dtype)
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
pass `vectorizer_dtype`

diff_hunk: @@ -1068,17 +1068,21 @@ def fit(self, X, y=None):
         """
         if not sp.issparse(X):
             X = sp.csc_matrix(X)
+
         if self.use_idf:
             n_samples, n_features = X.shape
-            df = _document_frequency(X)
+            # _document_frequency will return int32
+            # array by default, hence the cast
+            df = _document_frequency(X).astype(X.dtype
+                                               if X.dtype in FLOAT_DTYPES
+                                               else np.float64)
 
             # perform idf smoothing if required
             df += int(self.smooth_idf)
             n_samples += int(self.smooth_idf)
-
             # log+1 instead of log makes sure terms with zero idf don't get
             # suppressed entirely.
-            idf = np.log(float(n_samples) / df) + 1.0
+            idf = np.log(n_samples / df) + 1.0
             self._idf_diag = sp.spdiags(idf, diags=0, m=n_features,
path: sklearn/feature_extraction/text.py
text: @user2:
As mentioned by @user1 

```python
self._idf_diag = sp.diags(idf, offsets=0,
                                      shape=(n_features, n_features),
                                      format='csr',
                                      dtype=X.dtype
                                      if X.dtype in FLOAT_DTYPES
                                      else np.float64)
```

diff_hunk: @@ -995,3 +998,34 @@ def test_vectorizer_string_object_as_input():
             ValueError, message, vec.fit, "hello world!")
         assert_raise_message(
             ValueError, message, vec.transform, "hello world!")
+
+
+@pytest.mark.parametrize("dtype", [
+    np.float32,
+    np.float64
+])
+def test_tfidf_transformer_type(dtype):
+    # create made up data with parameterized dtype
+    X = rand(10, 20000, dtype=dtype, random_state=42)
+    X_trans = TfidfTransformer().fit_transform(X)
+    # check if the required data types match
+    assert X.dtype == X_trans.dtype
path: sklearn/feature_extraction/tests/test_text.py
text: @user1:
Usually we write `x == expected`

```python
assert X_trans.dtype == X.dtype
```