-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ENH: stats: end-to-end array-API support for NHSTs with beta null distribution #20793
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
|
||
def __betaincc(a, b, x): | ||
# not perfect; might want to just rely on SciPy | ||
return betainc(b, a, 1-x) |
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.
@steppi This is supposed to implement betaincc
in terms of betainc
for array backends that have betainc
but not betaincc
. I imagine that when x
is close to 1
, 1 - betainc(a, b, x)
might be better for unusual values of a
and b
. I thought I'd check with you what to do here: is this good enough, is there something better we can do (possibly involving evaluating both versions and choosing the better one), or should we not attempt to provide this sort of implementation?
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.
It's not going to work well or at all for small x
, but I see no issues for now since the dispatch to alternative backends is only enabled when the experimental SCIPY_ARRAY_API
flag is set. By the time what's now hidden behind SCIPY_ARRAY_API=1
becomes the default behavior, we should have an implementation of betaincc
that other backends can use. I don't think the values of a
and b
for which a beta distribution's pdf has a sharp peak at x=1
are that relevant for these NHSTs, so the time being this should work well enough for your use case on backends that don't support betaincc
.
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.
Could this implementation and the naive 1-betainc be complementary? Like if the result is close to 1, then 1-betainc should be fine, right?
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.
Never mind I suppose the result is close to zero precisely when x is close to 1, so neither approach would have good precision.
But yeah, I think that by the time this is no longer experimental, all the array libraries will support betaincc
.
@@ -81,11 +81,30 @@ def __xlogy(x, y, *, xp=xp): | |||
return __xlogy | |||
|
|||
|
|||
def _chdtr(xp, spx): |
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 basically just a copy-paste of _chdtrc
, so I thought it would be simple enough to include here. It isn't the main point of this PR, though, so I can take it out if preferred.
Since it's copy-paste, this begs the question of whether we can share code. Rather than trying that now, I think we should do a few more of these to see how it should be factored.
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.
LGTM once/if https://github.com/scipy/scipy/pull/20793/files#r1614998497 is resolved @steppi
Reference issue
Toward gh-20544
Follow-up to gh-20777
What does this implement/fix?
Similar to gh-20777 but for hypothesis tests with a beta null distribution.
Additional information
I also added
cdf
to_SimpleChi2
and used it incombine_pvalues
.