Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] 32/64-bit float consistency with BernoulliRBM #16352

Merged
merged 14 commits into from Jun 23, 2020
2 changes: 1 addition & 1 deletion doc/whats_new/v0.23.rst
Expand Up @@ -169,7 +169,7 @@ Changelog
deprecated. It has no effect. :pr:`11950` by
:user:`Jeremie du Boisberranger <jeremiedbb>`.

- |API| The ``random_state`` parameter has been added to
- |API| The ``random_state`` parameter has been added to
:class:`cluster.AffinityPropagation`. :pr:`16801` by :user:`rcwoolston`
and :user:`Chiara Marmo <cmarmo>`.

Expand Down
4 changes: 4 additions & 0 deletions doc/whats_new/v0.24.rst
Expand Up @@ -166,6 +166,10 @@ Changelog
:pr:`17603`, :pr:`17604`, :pr:`17606`, :pr:`17608`, :pr:`17609`, :pr:`17633`
by :user:`Alex Henrie <alexhenrie>`.

- |Enhancement| Avoid converting float32 input to float64 in
:class:`neural_network.BernoulliRBM`.
:pr:`16352` by :user:`Arthur Imbert <Henley13>`.

:mod:`sklearn.preprocessing`
............................

Expand Down
16 changes: 10 additions & 6 deletions sklearn/neural_network/_rbm.py
Expand Up @@ -131,7 +131,7 @@ def transform(self, X):
"""
check_is_fitted(self)

X = check_array(X, accept_sparse='csr', dtype=np.float64)
X = check_array(X, accept_sparse='csr', dtype=(np.float64, np.float32))
return self._mean_hiddens(X)

def _mean_hiddens(self, v):
Expand Down Expand Up @@ -344,16 +344,20 @@ def fit(self, X, y=None):
self : BernoulliRBM
The fitted model.
"""
X = self._validate_data(X, accept_sparse='csr', dtype=np.float64)
X = self._validate_data(
X, accept_sparse='csr', dtype=(np.float64, np.float32)
)
n_samples = X.shape[0]
rng = check_random_state(self.random_state)

self.components_ = np.asarray(
rng.normal(0, 0.01, (self.n_components, X.shape[1])),
order='F')
self.intercept_hidden_ = np.zeros(self.n_components, )
self.intercept_visible_ = np.zeros(X.shape[1], )
self.h_samples_ = np.zeros((self.batch_size, self.n_components))
order='F',
dtype=X.dtype)
self.intercept_hidden_ = np.zeros(self.n_components, dtype=X.dtype)
self.intercept_visible_ = np.zeros(X.shape[1], dtype=X.dtype)
self.h_samples_ = np.zeros((self.batch_size, self.n_components),
dtype=X.dtype)

n_batches = int(np.ceil(float(n_samples) / self.batch_size))
batch_slices = list(gen_even_slices(n_batches * self.batch_size,
Expand Down
44 changes: 43 additions & 1 deletion sklearn/neural_network/tests/test_rbm.py
@@ -1,9 +1,11 @@
import sys
import re
import pytest

import numpy as np
from scipy.sparse import csc_matrix, csr_matrix, lil_matrix
from sklearn.utils._testing import (assert_almost_equal, assert_array_equal)
from sklearn.utils._testing import (assert_almost_equal, assert_array_equal,
assert_allclose)

from sklearn.datasets import load_digits
from io import StringIO
Expand Down Expand Up @@ -189,3 +191,43 @@ def test_sparse_and_verbose():
r" time = (\d|\.)+s", s)
finally:
sys.stdout = old_stdout


@pytest.mark.parametrize("dtype_in, dtype_out", [
(np.float32, np.float32),
(np.float64, np.float64),
(np.int, np.float64)])
def test_transformer_dtypes_casting(dtype_in, dtype_out):
X = Xdigits[:100].astype(dtype_in)
rbm = BernoulliRBM(n_components=16, batch_size=5, n_iter=5,
random_state=42)
Xt = rbm.fit_transform(X)

# dtype_in and dtype_out should be consistent
assert Xt.dtype == dtype_out, ('transform dtype: {} - original dtype: {}'
.format(Xt.dtype, X.dtype))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While #16290 is not merged, we also should add a check that the results of fit_transform are close enough with float32 and float64 input.

Copy link
Contributor

@ksslng ksslng Feb 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also should check that the attributes of the estimator are close on float32 and float64 inputs, which I think is better to be done in the individual tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added a dedicated test for that.



def test_convergence_dtype_consistency():
# float 64 transformer
X_64 = Xdigits[:100].astype(np.float64)
rbm_64 = BernoulliRBM(n_components=16, batch_size=5, n_iter=5,
random_state=42)
Xt_64 = rbm_64.fit_transform(X_64)

# float 32 transformer
X_32 = Xdigits[:100].astype(np.float32)
rbm_32 = BernoulliRBM(n_components=16, batch_size=5, n_iter=5,
random_state=42)
Xt_32 = rbm_32.fit_transform(X_32)

# results and attributes should be close enough in 32 bit and 64 bit
assert_allclose(Xt_64, Xt_32,
rtol=1e-06, atol=0)
assert_allclose(rbm_64.intercept_hidden_, rbm_32.intercept_hidden_,
rtol=1e-06, atol=0)
assert_allclose(rbm_64.intercept_visible_, rbm_32.intercept_visible_,
rtol=1e-05, atol=0)
assert_allclose(rbm_64.components_, rbm_32.components_,
rtol=1e-03, atol=0)
assert_allclose(rbm_64.h_samples_, rbm_32.h_samples_)