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.

Sign up for GitHub

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

TST: stats.ttest_1samp: fix xslow test #20642

Merged
merged 2 commits into from
May 5, 2024
Merged

TST: stats.ttest_1samp: fix xslow test #20642

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa257e0
TST: stats.ttest_1samp: fix xslow test
mdhaber May 4, 2024
43db12f
TST: stats.ttest_1samp: adjust float generation strategy
mdhaber May 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions scipy/stats/tests/test_stats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3711,8 +3711,8 @@ def test_1samp_ci_iv(self, xp):
def test_pvalue_ci(self, alpha, data_axis, alternative, xp):
# test relationship between one-sided p-values and confidence intervals
data, axis = data_axis
data = data.astype(copy=True) # ensure byte order
data = xp.asarray(data)
data = data.astype(np.float64, copy=True) # ensure byte order
data = xp.asarray(data, dtype=xp.float64)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
data = data.astype(np.float64, copy=True) # ensure byte order
data = xp.asarray(data, dtype=xp.float64)
data = data.astype(np.float64, copy=True) # ensure byte order
# `float64` is needed here because of the range of `alpha`
data = xp.asarray(data, dtype=xp.float64)

Copy link
Contributor

Choose a reason for hiding this comment

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

I did check that all these incantations pass locally on x86_64 Linux, while they fail on main:

SCIPY_XSLOW=1 SCIPY_DEVICE=cuda python dev.py test -j 32 -b all -- -k "test_pvalue_ci"
SCIPY_XSLOW=1 python dev.py test -j 32 -b all -- -k "test_pvalue_ci"
SCIPY_XSLOW=1 python dev.py test -j 32 -- -k "test_pvalue_ci"

I tried to play picky reviewer and force float32 instead of float64 locally, and saw overflow/invalid value warnings, so that seems like a plausible justification for non-stats expert at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

(obivously the first one has limited value, since it is cpu only)

Copy link
Member

@lucascolley lucascolley May 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Am I missing something with the "ensure byte order" comment? Can we not just include the copy in asarray?

Suggested change
data = data.astype(np.float64, copy=True) # ensure byte order
data = xp.asarray(data, dtype=xp.float64)
# `float64` is needed here because of the range of `alpha`
data = xp.asarray(data, dtype=xp.float64, copy=True)

Copy link
Contributor Author

@mdhaber mdhaber May 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

hypothesis generates them in some nonstandard format that torch.asarray won't accept. Might be possible to make it generate more standard float64s.

Copy link
Member

Choose a reason for hiding this comment

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

okay, if that isn't possible then I think this looks okay. Maybe worth adding a comment as to why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I adjusted the test strategy to generate normal float64s. I think that the old strategy must have been left over from before the test took its final form.

res = stats.ttest_1samp(data, 0.,
alternative=alternative, axis=axis)
l, u = res.confidence_interval(confidence_level=alpha)
Expand All @@ -3722,7 +3722,7 @@ def test_pvalue_ci(self, alpha, data_axis, alternative, xp):
res = stats.ttest_1samp(data, popmean, alternative=alternative, axis=axis)
shape = list(data.shape)
shape.pop(axis)
ref = xp.broadcast_to(xp.asarray(1-alpha), shape)
ref = xp.broadcast_to(xp.asarray(1-alpha, dtype=xp.float64), shape)
xp_assert_close(res.pvalue, ref)


Expand Down
Loading