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

ENH: stats.ttest_1samp: add array-API support #20545

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented Apr 21, 2024

Reference issue

gh-20544

What does this implement/fix?

This adds array-API support to scipy.stats.ttest_1samp.

Additional Information

import numpy as np
import numpy as xp
from scipy import stats
rng = np.random.default_rng(34598234592345)
x = xp.asarray(rng.random((1000, 1000)))
stats.ttest_1samp(x, xp.asarray(0.))

# main (NumPy)
# 6.58 ms ± 299 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
# pr (NumPy)
# 6.59 ms ± 135 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
# pr (PyTorch)
# 3.68 ms ± 172 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

@mdhaber mdhaber added scipy.stats enhancement A new feature or improvement array types Items related to array API support and input array validation (see gh-18286) labels Apr 21, 2024
@rgommers
Copy link
Member

Nice! Would it make sense to post a basic benchmark in PR descriptions, to show things work with similar or improved speed compared to using NumPy? E.g., using CuPy if GPU is supported, and PyTorch CPU tensors if only other CPU implementations are supported?

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

took a look on my phone, may have missed something but looks pretty good!

scipy/stats/_stats_py.py Outdated Show resolved Hide resolved
scipy/stats/_stats_py.py Show resolved Hide resolved
scipy/stats/tests/test_stats.py Outdated Show resolved Hide resolved
scipy/stats/tests/test_stats.py Outdated Show resolved Hide resolved
@lucascolley
Copy link
Member

I am without a GPU for a while now, and I think all of these PRs should run CuPy through the test suite before going in.

@mdhaber
Copy link
Contributor Author

mdhaber commented Apr 21, 2024

Like pearsonr, this one is for CPU only until we have a standard way to convert to NumPy arrays for internal calculations (#18286 (comment)).

@mdhaber
Copy link
Contributor Author

mdhaber commented Apr 21, 2024

Addressed the comments and posted timing comparison at the top: NumPy performance is unchanged but PyTorch is considerably faster. One of the Windows tests stopped like:

........................................................................ [ 87%]
..............s...................................................s..... [ 87%]
...................................................
Error: The operation was canceled.

last time. Not seeing this in other PRs so let's watch out for it.

@rgommers
Copy link
Member

NumPy performance is unchanged but PyTorch is considerably faster.

Very nice. Even if PyTorch had not been faster this change has value - but almost 2x faster out of the box is great to see.

Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

All LGTM. I'm confident that test coverage hasn't reduced and that there aren't any spooky stats changes that have gone over my head. Thanks Matt!

@lucascolley lucascolley merged commit 3aba7ab into scipy:main Apr 22, 2024
30 checks passed
@lucascolley lucascolley added this to the 1.14.0 milestone Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
array types Items related to array API support and input array validation (see gh-18286) enhancement A new feature or improvement scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants