min, max and nans

min, max and nans

[Kevin Ryde]

While nosing around mpz_cmp_d in min and max, I noticed

	guile> (max +nan.0 0)
	+nan.0
	guile> (max 0 +nan.0)
	0.0

which strikes me as a bit inconsistent.

Is there a theory on what min and max should do with nans?  I'd think
returning nan if any argument is a nan would be a good idea.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Marius Vollmer]

Kevin Ryde <user42@zip.com.au> writes:

> Is there a theory on what min and max should do with nans?  I'd think
> returning nan if any argument is a nan would be a good idea.

That's what I think, too.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Kevin Ryde]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Marius Vollmer <mvo@zagadka.de> writes:
>
> That's what I think, too.

        * numbers.c (scm_max, scm_min): For inum, bignum and real, if other
        operand is NaN, then return NaN.  Also avoid passing NaN to mpz_cmp_d.

        * tests/numbers.test (max, min): Add tests involving NaNs.

Some cases just need the sense of the > or < test put the right way to
allow for it being false when involving a NaN.  A couple are right
already.  For bignums and for real/real an actual isnan is necessary.


[-- Attachment #2: numbers.c.min-max-nan.diff --]
[-- Type: text/plain, Size: 3908 bytes --]

--- numbers.c.~1.188.~	2003-05-13 09:14:53.000000000 +1000
+++ numbers.c	2003-05-26 17:42:15.000000000 +1000
@@ -2806,7 +2806,8 @@
       return (sgn < 0) ? x : y;
     } else if (SCM_REALP (y)) {
       double z = xx;
-      return (z <= SCM_REAL_VALUE (y)) ? y : scm_make_real (z);
+      /* if y==NaN then ">" is false and we return NaN */
+      return (z > SCM_REAL_VALUE (y)) ? scm_make_real (z) : y;
     } else {
       SCM_WTA_DISPATCH_2 (g_max, x, y, SCM_ARGn, s_max);
     }
@@ -2820,7 +2821,11 @@
       scm_remember_upto_here_2 (x, y);
       return (cmp > 0) ? x : y;
     } else if (SCM_REALP (y)) {
-      int cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (x), SCM_REAL_VALUE (y));
+      double yy = SCM_REAL_VALUE (y);
+      int cmp;
+      if (xisnan (yy))
+        return y;
+      cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (x), yy);
       scm_remember_upto_here_1 (x);
       return (cmp > 0) ? x : y;
     } else {
@@ -2829,13 +2834,23 @@
   } else if (SCM_REALP (x)) {
     if (SCM_INUMP (y)) {
       double z = SCM_INUM (y);
+      /* if x==NaN then "<" is false and we return NaN */
       return (SCM_REAL_VALUE (x) < z) ? scm_make_real (z) : x;
     } else if (SCM_BIGP (y)) {
-      int cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (y), SCM_REAL_VALUE (x));
+      double xx = SCM_REAL_VALUE (x);
+      int cmp;
+      if (xisnan (xx))
+        return x;
+      cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (y), SCM_REAL_VALUE (x));
       scm_remember_upto_here_1 (y);
       return (cmp < 0) ? x : y;
     } else if (SCM_REALP (y)) {
-      return (SCM_REAL_VALUE (x) < SCM_REAL_VALUE (y)) ? y : x;
+      /* if x==NaN then our explicit check means we return NaN
+         if y==NaN then ">" is false and we return NaN
+         a call to isnan is unavoidable, since it's the only way to know
+         which of x or y causes any compares to be false */
+      double xx = SCM_REAL_VALUE (x);
+      return (xisnan (xx) || xx > SCM_REAL_VALUE (y)) ? x : y;
     } else {
       SCM_WTA_DISPATCH_2 (g_max, x, y, SCM_ARGn, s_max);
     }
@@ -2872,6 +2887,7 @@
       return (sgn < 0) ? y : x;
     } else if (SCM_REALP (y)) {
       double z = xx;
+      /* if y==NaN then "<" is false and we return NaN */
       return (z < SCM_REAL_VALUE (y)) ? scm_make_real (z) : y;
     } else {
       SCM_WTA_DISPATCH_2 (g_min, x, y, SCM_ARGn, s_min);
@@ -2886,7 +2902,11 @@
       scm_remember_upto_here_2 (x, y);
       return (cmp > 0) ? y : x;
     } else if (SCM_REALP (y)) {
-      int cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (x), SCM_REAL_VALUE (y));
+      double yy = SCM_REAL_VALUE (y);
+      int cmp;
+      if (xisnan (yy))
+        return y;
+      cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (x), yy);
       scm_remember_upto_here_1 (x);
       return (cmp > 0) ? y : x;
     } else {
@@ -2895,13 +2915,23 @@
   } else if (SCM_REALP (x)) {
     if (SCM_INUMP (y)) {
       double z = SCM_INUM (y);
-      return (SCM_REAL_VALUE (x) <= z) ? x : scm_make_real (z);
+      /* if x==NaN then ">" is false and we return NaN */
+      return (SCM_REAL_VALUE (x) > z) ? scm_make_real (z) : x;
     } else if (SCM_BIGP (y)) {
-      int cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (y), SCM_REAL_VALUE (x));
+      double xx = SCM_REAL_VALUE (x);
+      int cmp;
+      if (xisnan (xx))
+        return x;
+      cmp = xmpz_cmp_d (SCM_I_BIG_MPZ (y), xx);
       scm_remember_upto_here_1 (y);
       return (cmp < 0) ? y : x;
     } else if (SCM_REALP (y)) {
-      return (SCM_REAL_VALUE (x) < SCM_REAL_VALUE (y)) ? x : y;
+      /* if x==NaN then our explicit check means we return NaN
+         if y==NaN then "<" is false and we return NaN
+         a call to isnan is unavoidable, since it's the only way to know
+         which of x or y causes any compares to be false */
+      double xx = SCM_REAL_VALUE (x);
+      return (xisnan (xx) || xx < SCM_REAL_VALUE (y)) ? x : y;
     } else {
       SCM_WTA_DISPATCH_2 (g_min, x, y, SCM_ARGn, s_min);
     }

[-- Attachment #3: numbers.test.min-max-nan.diff --]
[-- Type: text/plain, Size: 2664 bytes --]

--- numbers.test.~1.23.~	2003-05-10 13:01:51.000000000 +1000
+++ numbers.test	2003-05-26 17:45:08.000000000 +1000
@@ -1646,6 +1646,9 @@
 ;;;
 
 (with-test-prefix "max"
+  (pass-if (= 456.0 (max 123.0 456.0)))
+  (pass-if (= 456.0 (max 456.0 123.0)))
+
   (let ((big*2 (* fixnum-max 2))
         (big*3 (* fixnum-max 3))
         (big*4 (* fixnum-max 4))
@@ -1654,7 +1657,15 @@
     (pass-if (= +inf.0 (max big*5  +inf.0)))
     (pass-if (= +inf.0 (max +inf.0 big*5)))
     (pass-if (= big*5  (max big*5  -inf.0)))
-    (pass-if (= big*5  (max -inf.0 big*5))))
+    (pass-if (= big*5  (max -inf.0 big*5)))
+
+    (pass-if (nan? (max 123 +nan.0)))
+    (pass-if (nan? (max big*5 +nan.0)))
+    (pass-if (nan? (max 123.0 +nan.0)))
+    (pass-if (nan? (max +nan.0 123)))
+    (pass-if (nan? (max +nan.0 big*5)))
+    (pass-if (nan? (max +nan.0 123.0)))
+    (pass-if (nan? (max +nan.0 +nan.0))))
 
   ;; in gmp prior to 4.2, mpz_cmp_d ended up treating Inf as 2^1024, make
   ;; sure we've avoided that
@@ -1672,7 +1683,12 @@
 		  (1+ (ash 1 1024))
 		  (- (1- (ash 1 1024)))
 		  (- (ash 1 1024))
-		  (- (1+ (ash 1 1024))))))
+		  (- (1+ (ash 1 1024)))))
+
+  ;; in gmp prior to 4.2, mpz_cmp_d ended up treating NaN as 3*2^1023, make
+  ;; sure we've avoided that
+  (pass-if (nan? (max (ash 1 2048) +nan.0)))
+  (pass-if (nan? (max +nan.0 (ash 1 2048)))))
 
 ;;;
 ;;; min
@@ -1681,6 +1697,9 @@
 ;; FIXME: unfinished...
 
 (with-test-prefix "min"
+  (pass-if (= 123.0 (min 123.0 456.0)))
+  (pass-if (= 123.0 (min 456.0 123.0)))
+
   (let ((big*2 (* fixnum-max 2))
         (big*3 (* fixnum-max 3))
         (big*4 (* fixnum-max 4))
@@ -1706,7 +1725,15 @@
     (pass-if (= big*5  (min big*5  +inf.0)))
     (pass-if (= big*5  (min +inf.0 big*5)))
     (pass-if (= -inf.0 (min big*5  -inf.0)))
-    (pass-if (= -inf.0 (min -inf.0 big*5))))
+    (pass-if (= -inf.0 (min -inf.0 big*5)))
+
+    (pass-if (nan? (min 123 +nan.0)))
+    (pass-if (nan? (min big*5 +nan.0)))
+    (pass-if (nan? (min 123.0 +nan.0)))
+    (pass-if (nan? (min +nan.0 123)))
+    (pass-if (nan? (min +nan.0 big*5)))
+    (pass-if (nan? (min +nan.0 123.0)))
+    (pass-if (nan? (min +nan.0 +nan.0))))
   
   ;; in gmp prior to 4.2, mpz_cmp_d ended up treating Inf as 2^1024, make
   ;; sure we've avoided that
@@ -1724,7 +1751,12 @@
 		  (1+ (ash 1 1024))
 		  (- (1- (ash 1 1024)))
 		  (- (ash 1 1024))
-		  (- (1+ (ash 1 1024))))))
+		  (- (1+ (ash 1 1024)))))
+
+  ;; in gmp prior to 4.2, mpz_cmp_d ended up treating NaN as 3*2^1023, make
+  ;; sure we've avoided that
+  (pass-if (nan? (min (- (ash 1 2048)) (- +nan.0))))
+  (pass-if (nan? (min (- +nan.0) (- (ash 1 2048))))))
 
 ;;;
 ;;; +

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Kevin Ryde]

I wrote:
>         * numbers.c (scm_max, scm_min): For inum, bignum and real, if other
>         operand is NaN, then return NaN.  Also avoid passing NaN to mpz_cmp_d.
>
>         * tests/numbers.test (max, min): Add tests involving NaNs.

I made this change.  Actually, I had a bit of a brain fade and added
the tests the other day, but not the code.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Marius Vollmer]

Kevin Ryde <user42@zip.com.au> writes:

> I made this change.  Actually, I had a bit of a brain fade and added
> the tests the other day, but not the code.

:-) I noticed because the snapshots didn't build successfully, but I
thought I'd give you a day or two before sending you the build log...

Should I make the build logs available as well?  In what form?  For a
short time, they were posted to guile-cvs@gnu.org.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Mikael Djurfeldt]

Marius Vollmer <mvo@zagadka.de> writes:

> Kevin Ryde <user42@zip.com.au> writes:
>
>> I made this change.  Actually, I had a bit of a brain fade and added
>> the tests the other day, but not the code.
>
> :-) I noticed because the snapshots didn't build successfully, but I
> thought I'd give you a day or two before sending you the build log...
>
> Should I make the build logs available as well?  In what form?  For a
> short time, they were posted to guile-cvs@gnu.org.

In my opinion, the idea of sending build logs at failure (not when
Guile compiles and tests OK) to guile-cvs@gnu.org is an excellent
idea.

If I have at any time been critical against it, it was because it is a
good idea to start off with no build failures so that people feel
motivated to fix things.

M


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Marius Vollmer]

Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:

> In my opinion, the idea of sending build logs at failure (not when
> Guile compiles and tests OK) to guile-cvs@gnu.org is an excellent
> idea.
> 
> If I have at any time been critical against it, it was because it is a
> good idea to start off with no build failures so that people feel
> motivated to fix things.

Hehe, yeah, I could totally understand your point (if I understood it
;-).  For those interested: the snapshots were failing for over a week
due to problems in the build environment (I think).  I didn't get
around to fix the build environment in a timely manner and so people
saw error logs that were not relevant to them.  That could only teach
them to ignore these logs.  So I stopped posting them automatically.

We _did_ start off with no build failures but that was hard to notice
since no logs are sent when there is no build failure.

I think it is also OK when just I myself watch the logs and contact
the people that I deem responsible for failures.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Kevin Ryde]

I notice C99 fmin and fmax return the other operand when one is a NaN,
and return NaN only when both are NaNs.

I tend to think of nan as indicating something evil, and it should be
propagated.  But the c99 way would be a possibility to consider.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: min, max and nans

[Marius Vollmer]

Kevin Ryde <user42@zip.com.au> writes:

> I notice C99 fmin and fmax return the other operand when one is a NaN,
> and return NaN only when both are NaNs.
> 
> I tend to think of nan as indicating something evil, and it should be
> propagated.  But the c99 way would be a possibility to consider.

Yep.  Does the IEEE standard say aomething about max and min?

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel