Repository navigation
[MRG] 32/64-bit float consistency with BernoulliRBM - #16352
Conversation
Henley13
changed the title
|
|
||
| # dtype_in and dtype_out consistent | ||
| assert Xt.dtype == dtype_out, ('transform dtype: {} - original dtype: {}' | ||
| .format(Xt.dtype, X.dtype)) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Just added a dedicated test for that.
jnothman
left a comment
jnothman
left a comment
There was a problem hiding this comment.
Is this more susceptible to loss of precision in intermediate matrix multiplications?
I guess this question could apply to any neural net architecture, and yet all DL libraries use float32 and sometimes even floa16 lately. Maybe they just don't enforce that outputs in f64/f32 are identical? Not sure. If so, I wonder whether we should. Possibly checking that the convergence critera was reached no matter what it was is enough (at least for some type of algorithms). |
| assert_almost_equal(Xt_64, Xt_32, 6) | ||
| assert_almost_equal(rbm_64.intercept_hidden_, | ||
| rbm_32.intercept_hidden_, | ||
| 6) | ||
| assert_almost_equal(rbm_64.intercept_visible_, | ||
| rbm_32.intercept_visible_, | ||
| 6) | ||
| assert_almost_equal(rbm_64.components_, rbm_32.components_, 6) | ||
| assert_almost_equal(rbm_64.h_samples_, rbm_32.h_samples_, 0) |
There was a problem hiding this comment.
Please use assert_allclose instead
| h : ndarray of shape (n_samples, n_components) | ||
| Corresponding mean field values for the hidden layer. | ||
| """ | ||
|
|
|
|
||
| p = safe_sparse_dot(v, self.components_.T) | ||
| p += self.intercept_hidden_ | ||
|
|
| :class:`neural_network.MLPClassifier` by clipping the probabilities. | ||
| :pr:`16117` by `Thomas Fan`_. | ||
|
|
||
| - |Enhancement| Prevent the transformer from converting float32 to float64 in |
There was a problem hiding this comment.
Maybe "Avoid converting float32 input to float64 in ..."?
|
Thanks @Henley13 ! |
6 participants
Reference Issues/PRs
Works on #11000 for BernoulliRBM.
What does this implement/fix? Explain your changes.
Prevent the transformer from converting float32 to float64.
Any other comments?
Maybe we should wait to merge a generic test for dtype consistency (see #16290) before merging this one.