-
-
Notifications
You must be signed in to change notification settings - Fork 9
Implement isfinite, isinf, isnan ufuncs #121
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
base: main
Are you sure you want to change the base?
Conversation
I get some weird runtime warnings in the isfinite and isinf ufuncs:
Any idea what I'm doing wrong? |
IEEE says comparisons with NaNs trigger FPEs (floating point error; FPEs are a flag set on the CPU when one occurs basically). That means normally also |
This definitely got me on the right track but wasn't the issue in the end. In fact, |
Seems plausible that the sleef casts uses the wrong type of comparisons internally and nobody noticed/cares (or the compilers mess it up later). @juntyr one thing I forgot is, that we should definitely add |
d5a15f1
to
6e6a88b
Compare
@SwayamInSync I've rebased this PR and it should now also be good to go |
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 @juntyr this looks good, just a small suggestion here.
@@ -1,6 +1,12 @@ | |||
#include <sleef.h> | |||
#include <sleefquad.h> | |||
#include <cmath> | |||
#include <limits> | |||
|
|||
// Quad Constants, generated with qutil |
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 seems QUAD_ZERO
and QUAD_ONE
aren't used so you can remove them, also
sleef_q
internally handles the biased exponent addition which managed to convert this to IEEE validated infinity representation but I am kind of biased here to keep this hardcoded as it can be easily defined as Sleef_cast_from_doubleq1(1.0 / 0.0)
(Dividing a non-zero finite number by zero)
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.
Whoops, my rebase was messed up then as I originally used the constants
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.
I originally tried the above but converting from infinity double caused numpy warning to appear, which I was only able to remove with the constants
Fixes #111