fmod nan_mix usage

[enh]

On Mon, Jul 23, 2018 at 12:54 PM Bruce Evans <brde at optusnet.com.au> wrote:

> On Mon, 23 Jul 2018, enh via freebsd-numerics wrote:
>
> > the recent change from
> >
> >  return (x*y)/(x*y);
> >
> > to
> >
> >  return nan_mix(x, y)/nan_mix(x, y);
> >
> > in e_fmod.c broke some of our unit tests. for example, fmod(3.f, 0.f) in
> > one of the VM tests.
>
> This is a bug in my change.
>
> nan_mix(x, y) does essentially x+y, but here essentially x*y is needed so
> that y = 0 gives 0 unless x is NaN.  In the example, adding gives 3 instead
> of 0, so the final result is 1 instead of 0.0/0.0 = NaN.
>
> The log message mentions avoiding this problem in s_ccosh[fl].c and
> s_sinh[fl].c.  This list was supposed have all special cases.
>
> Unfortunately, this seems to prevent use of a single macro.  I will try
> using a 2 macros with 1 using sums and the other products.  The non-broken
> cases converted sums to sums.
>
> > bionic/tests/math_test.cpp:(784) Failure in test
> > math_h_force_long_double.fmod
> > Value of: isnan(fmod(3.0, 0.0))
> >  Actual: false
> > Expected: true
> > math_h_force_long_double.fmod exited with exitcode 1.
> > [  FAILED  ] math_h_force_long_double.fmodf (13 ms)
> > bionic/tests/math_test.cpp:(798) Failure in test
> > math_h_force_long_double.fmodf
> > Value of: isnanf(fmodf(3.0f, 0.0f))
> >  Actual: false
> > Expected: true
> > math_h_force_long_double.fmodf exited with exitcode 1.
> > [  FAILED  ] math_h_force_long_double.fmodl (12 ms)
> > bionic/tests/math_test.cpp:(812) Failure in test
> > math_h_force_long_double.fmodl
> > Value of: isnanl(fmodl(3.0L, 0.0L))
> >  Actual: false
> > Expected: true
>
> Do you have a lot of special tests like this?  I mostly use generic tests
> that don't assert any particular result, but compare the results in
> different precisions.  I apparently changed all precisions to be
> consistently wrong at the same time.
>

bionic doesn't have as many as it should, though i do add them any time we
catch a regression. all our tests are in
https://android.googlesource.com/platform/bionic/+/master/tests/ with
complex_test.cpp and math_test.cpp being the interesting ones.
(complex_test.cpp is laughably perfunctory right now, but sadly *did* catch
bugs where historically the makefiles were broken and we weren't shipping
all the functions for all the architectures.)

i do try to ensure that we use the BSD rather than APL2 license for the
tests, though apparently i failed in both these cases. i'll fix the headers
if that helps.

i don't think the contributed Android tests in math_data/ are very high
quality. i have no reason to believe they're not just random numbers (and
glibc fails several of them, and i don't know who's right in those cases).

the external/arm-optimized-routines project (corresponding to
https://github.com/ARM-software/optimized-routines) has much better tests
(for those functions they support) and clearly distinguish between directed
and random testing:
https://github.com/ARM-software/optimized-routines/tree/master/test/testcases


> > it looks like e_remainder.c might have the same issue, but Android's
> tests
> > didn't catch that :-( i'll improve the tests...
>
> Indeed.  Also remquo* and ctanh* :-(.  ctanh* should be more like csinh*
> and ccosh*, and it was.
>

yeah, i caught remquo after i hit send (and have just uploading a CL with
the missing tests). i'm glad to hear that ctanh* actually works because i'd
failed to break it :-) i'll commit those extra tests too anyway.

(strictly, the netbsd ctanhl we use is broken for the NaN+0i case,
returning NaN+NaNi rather than NaN+0i, but that's not your fault, other
than that i'll switch to the freebsd ld128 ctanhl as soon as it exists :-) )


> The only other complicated case seems to be hypot[fl]().  This subtracts
> instead of adds, since it wants to convert Inf-Inf to NaN.
>

hypot seems okay from my testing. am i missing another test?


> Bruce
>