[BUG] Preserve float precision in Numba helpers - #3726
Conversation
This pull request includes code written with the assistance of AI. The code has **not yet been reviewed** by a human.
Thank you for contributing to
|
|
Hi, thanks for working on this. I'll wait for #3707 to get in first, as we would want to re-use (or adapt) some functions declared there to initialize arrays with the correct dtype, instead of re-declaring the X / 1 trick everywhere. Other than that it looks correct, will review better when the other PR is in. |
|
hi @cakeni the first PR is in. Could you try to uniformize what you done to use the same reasoning ? |
This pull request includes code written with the assistance of AI.\nThe code has **not yet been reviewed** by a human.
|
Done — I’ve updated the dtype initialization to reuse |
baraline
left a comment
baraline
left a comment
There was a problem hiding this comment.
Just one small comment on the test data and we can merge this, good job !
| @pytest.mark.parametrize("dtype", DATATYPES) | ||
| def test_float_output_dtype_follows_input_precision(dtype): | ||
| """Test helpers preserve float precision and promote integer input.""" | ||
| X = np.arange(24, dtype=dtype).reshape(2, 12) |
There was a problem hiding this comment.
Could you just replace this with the testing.utils function to generate data ? (and cast it to the dtype)
| """ | ||
| n_channels, _ = X.shape | ||
| values = np.zeros((n_channels, length)) | ||
| values = np.zeros((n_channels, length), dtype=_as_normalised_float(X[:0]).dtype) |
There was a problem hiding this comment.
It's not "pretty", but doing things like :
def _as_normalised_float_dtype(input_dtype):
if input_dtype == np.float32 return np.float64 else return np.float32
Is not possible inside numba functions (or at least not without overcomplicating things), so we'll go with this for now.
3 participants
Reference Issues/PRs
Fixes #3720. See also #3707, which addresses the same dtype class in separate z-normalisation helpers.
What does this implement/fix? Explain your changes.
Allocates outputs using the dtype produced by true division of the input. This keeps
float32inputs asfloat32across all six affected Numba helpers while preserving the existingfloat64output for integer andfloat64inputs. A focused regression test covers every returned array from the six helpers.The calculation loops are unchanged; only output allocation dtype is corrected. If #3707 merges first, its changes touch the same module but this fix does not depend on them.
Does your contribution introduce a new dependency? If yes, which one?
No.
Any other comments?
Validation performed locally:
.venv\Scripts\python.exe -m pytest -o addopts='' aeon/utils/numba/tests/test_general.py -q— 42 passed.venv\Scripts\python.exe -m pytest -o addopts='' aeon/utils/numba/tests/test_general.py -q --nonumba true— 42 passed.venv\Scripts\python.exe -m pytest -o addopts='' aeon/distances/elastic/tests/test_distance_correctness.py -q— 2 passed.venv\Scripts\python.exe -m pytest -o addopts='' aeon/similarity_search/subsequence/tests/test_mass.py -q— 9 passed.venv\Scripts\pre-commit.exe run --files aeon/utils/numba/general.py aeon/utils/numba/tests/test_general.py— all hooks passedPR checklist
For all contributions
@all-contributorsafter this PR has been merged.This pull request includes code written with the assistance of AI.
The code has not yet been reviewed by a human.