FEAT: nextafter ufunc implementation
#190
Conversation
|
Tagging @seberg @ngoldbaum @juntyr please review this one. I also check the NumPy's implementation but in the comments seem to mention faults there so I skipped it. |
|
|
||
| // NaN if either is NaN | ||
| if (Sleef_iunordq1(*x, *y)) { | ||
| return Sleef_addq1_u05(*x, *y); // still NaN |
There was a problem hiding this comment.
We can also just return the NaN here but libquadmath's version was doing addition to I did that too just to keep the consistency between implementtion
The thing is addition will also return NaN but the internal bit representation would be changed. Its a call of choice I think, if you all prefer this we can keep it.
|
What does nextafter(-0.0, 1.0) and nextafter(+0.0, -1.0) do? |
|
Spelling error (but for some reason I cannot comment inline right now): "did not need the checks" (at the final sleef return) |
|
Once we also have an implementation of the spacing ufunc, both ufuncs should have a test using the other ufunc |
|
Thanks @SwayamInSync! You have done so much fantastic work over the past few days! |
nextafter(-0, 1) => (+) smallest subnormal Its like 1st argument defines where you are and 2nd in which direction (not value) you want to go and returns the very next representable floating value
Ah sorry, will fix that |
For |
|
Spacing, you may have to be careful about the sign of x, but not sure if NumPy proper does that right. Both since spacing is presumably always absolute and because the larger spacing is away from zero. |
In numpy sign depends on the values, I am not sure whether this is the right behaviour, as from the docs which mention Distance should always be positive. So what you recommend? following NumPy's behaviour or change it to always return positive (this will also require to be fix from NumPy side then) |
|
Yeah, I thought I faintly remembered there was something weird about it :(. So it uses the right distance (the larger one), but returns a signed value... While it seems to me that an absolute value makes more sense, it is very unclear to me if someone might be relying on it being signed!? The docs also feel slightly unclear, since it is actually not the nearest adjacent number, it is the second nearest (and that is good). In the end... I guess you may want to just follow NumPy in its current behavior and update the docs :/. I would like to change it, but I am not sure it is worthwhile, unless we do it by renaming the fundction or so... |
|
Cool taking this in! |
3 participants
This PR adds the
nextafterufunc support to quaddtype. I referenced the gcc's libquadmath's implementation for __float128 and tried to map it back in SLEEF.For testing we can't do the normal value comparison against numpy with other dtype like float64 so I added checking the properties like
and similar more checking symmetry and direction. I also tested the C implementation against the libquadmath and it seems to be passing there as well.