[quickjs-devel] Re: [PATCH] fix js_math_hypot

  • From: Diego Nehab <diego.nehab@xxxxxxxxx>
  • To: quickjs-devel@xxxxxxxxxxxxx
  • Date: Mon, 24 Feb 2020 21:10:38 -0300

I think we both understand that there was a bug in the implementation. I
simply meant to alert the list to the fact that, even with the fix, the
implementation does not do what hypot is supposed to do (although it is
much closer!)

On Mon, Feb 24, 2020 at 8:54 PM ian <ian@xxxxxxxxx> wrote:

Hi...

Please you both, read carefully.
The point was not to return the square root or not....

It was about correcting sum(a) to sum(a*a) ....

-            r += a;
+            r += a*a;
E.g. : for js_mat_hypot (a,b) => sqrt(a²+b²) instead of sqrt(a+b) ....

Regards,

ian
Le 24/02/2020 à 20:50, Diego Nehab a écrit :

I think the sole motivation behind the existence of a library function
hypot is exactly that "Note". The bullets come almost for free in the naive
implementation.

On Mon, Feb 24, 2020 at 4:14 PM Wes Garland <wes@kingsds.network>
<wes@kingsds.network> wrote:

The current ES spec leaves this as "implementation-defined" and does not
discuss ULP at all -- except to recommend (but not require) that the
implementation back the routines in Math with fdlibm.

That said, I believe this implementation is deficient in terms of the
spec   (based on 30 seconds of code review, not deep thought), at least for
the cases where the arguments are +0 or -0.  It also violates the "care
should be taken...." note, but I see that as less critical.

Math.hypot returns an implementation-dependent approximation of the
square root of the sum of squares of its arguments.

   - If no arguments are passed, the result is +0.
   - If any argument is +∞, the result is +∞.
   - If any argument is -∞, the result is +∞.
   - If no argument is +∞ or -∞, and any argument is NaN, the result is
   NaN.
   - If all arguments are either +0 or -0, the result is +0.

Note
Implementations should take care to avoid the loss of precision from
overflows and underflows that are prone to occur in naive implementations
when this function is called with two or more arguments.

Wes

On Sat, Feb 22, 2020 at 4:14 PM Diego Nehab <diego.nehab@xxxxxxxxx>
wrote:

I don’t know about the details of the JS hypot function, but in C and
C++ it usually guarantees the precision of less than 1ulp.  It’s not simply
a convenience function to save typing. This involves, for example, finding
the element of greatest absolute value and factoring it out of the square
root.

https://en.m.wikipedia.org/wiki/Hypot


On Sat, Feb 22, 2020 at 15:29 Yunkang YU <yushroom@xxxxxxxxx> wrote:

Math.hypot() should return the square root of the sum of *squares* of
its arguments

--- a/quickjs.c
+++ b/quickjs.c
@@ -39923,7 +39923,7 @@ static JSValue js_math_hypot(JSContext *ctx,
JSValueConst this_val,
         for(i = 0; i < argc; i++) {
             if (JS_ToFloat64(ctx, &a, argv[i]))
                 return JS_EXCEPTION;
-            r += a;
+            r += a*a;
         }
         r = sqrt(r);
     }

--
Sent from my phone



--
Wesley W. Garland
CTO, Kings Distributed Systems
+1 613 539 6952

--
-- ian@xxxxxxxxx
-- Développeur compulsif

Other related posts: