-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
ENH: stats.tvar/tstd/tsem: add array API support #21036
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
Conversation
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.
can take a closer look later but all looks good from a scan
I will push again once I fix the torch issues and confirm locally. (I have torch and JAX set up on my Mac, not this Windows machine, which has CuPy.) |
[skip cirrus] [skip circle]
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.
Thanks LGTM!
The comment on line 63 of test_stats.py
is somewhat amusing!
def test_tmean(self, xp): | ||
x = xp.asarray(X) | ||
x = xp.asarray(X.tolist()) # use default dtype of xp |
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.
This is a neat trick I hadn't thought of doing this
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.
How portable is this out of interest?
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.
tolist
says it converts NumPy scalars to Python scalars, and it sounds like a Python float
is all but guaranteed to be an IEEE 754 double (also here). I know the standard doesn't specify everything, so maybe that's not enough to guarantee portability, but hopefully it's good enough for the precision we look for in the tests.
Reference issue
Toward gh-20544
What does this implement/fix?
Adds array API support to
tvar
,tstd
, andtsem
.