-
Notifications
You must be signed in to change notification settings - Fork 325
[BUG] Preserve float32 dtype in affected transformers. #3752
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
7a1e22e
b83634c
c4737dd
ee0267e
6818aa2
45504fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,8 @@ def _fit_transform(self, X, y=None): | |
| for i in range(1, self.n_intervals): | ||
| Xt = np.hstack((Xt, transformed_intervals[i])) | ||
|
|
||
| return Xt | ||
| out_dtype = np.float32 if X.dtype == np.float32 else np.float64 | ||
| return Xt.astype(dtype=out_dtype, copy=False) | ||
|
|
||
| def _fit(self, X, y=None): | ||
| X, y, rng = self._fit_setup(X, y) | ||
|
|
@@ -251,7 +252,8 @@ def _transform(self, X, y=None): | |
| for i in range(len(self.intervals_)) | ||
| ) | ||
|
|
||
| Xt = np.zeros((X.shape[0], len(transform))) | ||
| out_dtype = np.float32 if X.dtype == np.float32 else np.float64 | ||
| Xt = np.zeros((X.shape[0], len(transform)), dtype=out_dtype) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above, we want the interval computation to use 32 bit precision, not simply cast the output. So
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _transform_intervals now returns values in the working X.dtype, and _transform allocates its result directly with that dtype instead of correcting the dtype |
||
| for i, t in enumerate(transform): | ||
| Xt[:, i] = t | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| """Interval extraction test code.""" | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
|
|
||
| from aeon.testing.data_generation import make_example_3d_numpy | ||
| from aeon.transformations.collection.feature_based import Catch22, SevenNumberSummary | ||
| from aeon.transformations.collection.interval_based import ( | ||
|
|
@@ -56,3 +59,39 @@ def test_supervised_transformers(): | |
| X_t = sit.fit_transform(X, y) | ||
|
|
||
| assert X_t.shape == (X.shape[0], 8) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype, expected_dtype", | ||
| [ | ||
| (np.float32, np.float32), | ||
| (np.float64, np.float64), | ||
| (np.int64, np.float64), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to also add int32, float64
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| ], | ||
| ) | ||
| def test_supervised_intervals_preserves_float_precision(dtype, expected_dtype): | ||
| """Test SupervisedIntervals preserves float32 and promotes integer input.""" | ||
| X, y = make_example_3d_numpy( | ||
| random_state=0, | ||
| n_channels=1, | ||
| n_timepoints=20, | ||
| ) | ||
| X = X.astype(dtype) | ||
|
|
||
| sit = SupervisedIntervals( | ||
| features=[row_mean], | ||
| n_intervals=2, | ||
| random_state=0, | ||
| ) | ||
| sit.fit(X, y) | ||
| Xt = sit.transform(X) | ||
|
|
||
| sit = SupervisedIntervals( | ||
| features=[row_mean], | ||
| n_intervals=2, | ||
| random_state=0, | ||
| ) | ||
| Xt_fit_transform = sit.fit_transform(X, y) | ||
|
|
||
| assert Xt.dtype == expected_dtype | ||
| assert Xt_fit_transform.dtype == expected_dtype | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,3 +111,20 @@ def test_dwt_performs_correcly_along_each_dim(): | |
| ] | ||
| ) | ||
| np.testing.assert_array_almost_equal(res, orig) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype, expected_dtype", | ||
| [ | ||
| (np.float32, np.float32), | ||
| (np.float64, np.float64), | ||
| (np.int64, np.float64), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to before, add int32, float64
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
| ], | ||
| ) | ||
| def test_dwt_preserves_float_precision(dtype, expected_dtype): | ||
| """Check dwt preserves float32 and promotes proper dtype output.""" | ||
| X = np.arange(16, dtype=dtype).reshape(2, 1, 8) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use the testing utils functions to generate testing data, as you did in the inerval tests (make_example_3d_numpy)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now uses make_example_3d_numpy |
||
|
|
||
| Xt = DWTTransformer(n_levels=2).fit_transform(X) | ||
|
|
||
| assert Xt.dtype == expected_dtype | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,3 +75,23 @@ def test_slope_performs_correcly_along_each_dim(): | |
| ] | ||
| ) | ||
| np.testing.assert_array_almost_equal(res, orig, decimal=5) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype, expected_dtype", | ||
| [ | ||
| (np.float32, np.float32), | ||
| (np.float64, np.float64), | ||
| (np.int64, np.float64), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also int32, float64
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
| ], | ||
| ) | ||
| def test_slope_preserves_float_precision(dtype, expected_dtype): | ||
| """Check that Slope preserved float32 and promotes proper dtype output.""" | ||
| X = np.array( | ||
| [[[4, 6, 10, 12, 8, 6, 5, 5]]], | ||
| dtype=dtype, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, use the testing utils to generate data
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to use make_example_3d_numpy |
||
|
|
||
| Xt = SlopeTransformer(n_intervals=2).fit_transform(X) | ||
|
|
||
| assert Xt.dtype == expected_dtype | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,3 +87,23 @@ def test_incorrect_arguments(): | |
| resizer = Resizer(resized_length="invalid") | ||
| with pytest.raises(ValueError, match="resized_length must be"): | ||
| resizer.fit_transform(X) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "dtype, expected_dtype", | ||
| [ | ||
| (np.float32, np.float32), | ||
| (np.float64, np.float64), | ||
| (np.int64, np.float64), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also int32,float64 |
||
| ], | ||
| ) | ||
| def test_resizer_preserves_float_precision(dtype, expected_dtype): | ||
| """Test Resizer preserved float32 and promotes proper dtype output.""" | ||
| X = [ | ||
| np.array([[0, 1, 2, 3]], dtype=dtype), | ||
| np.array([[0, 2, 4, 6, 8]], dtype=dtype), | ||
|
Comment on lines
+99
to
+100
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the current testing util to generate variable length list allows dtype args, so this one is fine for now. |
||
| ] | ||
|
|
||
| Xt = Resizer(resized_length=6).fit_transform(X) | ||
|
|
||
| assert Xt.dtype == expected_dtype | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't neet to cast the input here, it must natively used float32 (or 64) in the generate intervals function.
What we want is that if we have a float 32 input, computation are made on float 32 to benefit from the reduced memory footprint (and computation time to some extent)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed final cast. The input is promoted once and interval buffers now use X.dtype, so float32 is preserved throughout the SupervisedIntervals.