-
Notifications
You must be signed in to change notification settings - Fork 134
Add betainc C implementation #798
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
Seems like you need to run pre-commit for auto-formatting |
Could you remove the comma after |
Thanks, that also led me to realize I didn't set a code cache version. I set |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #798 +/- ##
==========================================
+ Coverage 80.76% 80.83% +0.07%
==========================================
Files 163 163
Lines 46941 46907 -34
Branches 11499 11470 -29
==========================================
+ Hits 37912 37918 +6
+ Misses 6786 6772 -14
+ Partials 2243 2217 -26
|
pytensor/scalar/math.py
Outdated
raise NotImplementedError("type not supported", type) | ||
|
||
def c_code_cache_version(self): | ||
return (0, 0, 1) |
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.
Nitpick: return (1,)
?
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.
Looks great, just one nit
pytensor/scalar/math.py
Outdated
if node.inputs[0].type in float_types: | ||
return f"""{z} = BetaInc({a}, {b}, {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.
Don't all the inputs need to be floats?
Sorry for the delay, I addressed your comments. Thanks for catching the input check. |
Sorry for the delay @arthus701 and thanks a lot |
This reverts commit 31bf682.
Description
As discussed over at #673, this adds a C-implementation to BetaInc.
Related Issue
Type of change