Date: Mon, 23 Jul 2018 14:30:35 -0700 From: enh <enh@google.com> To: brde@optusnet.com.au Cc: freebsd-numerics@freebsd.org Subject: Re: fmod nan_mix usage Message-ID: <CAJgzZoq2Cjzsn8ogzM8YBsrdeszHzYPZq9X8Zbn%2Bo1rCWEpxkg@mail.gmail.com> In-Reply-To: <20180724050141.Q2280@besplex.bde.org> References: <CAJgzZopb_0fxM9jbVjUEZ0JPOfcrgeQo_Ki-afZ5aRNr38tKVg@mail.gmail.com> <20180724050141.Q2280@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 23, 2018 at 12:54 PM Bruce Evans <brde@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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJgzZoq2Cjzsn8ogzM8YBsrdeszHzYPZq9X8Zbn%2Bo1rCWEpxkg>