Mercurial > octave

changeset 16169:0303fda3e929

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix range behavior with -0 endpoints (bug #38423) * libinterp/interpfcn/pr-output.cc(octave_print_internal): print base or limit of range rather than using expression base+i*increment which can destroy the signbit of base/limit. * liboctave/array/Range.cc(Range constructor): Move trivial 2-line constructor to .h file. * liboctave/array/Range.cc(matrix_value, checkelem): Return base for first element of array. Return limit of range for end of array if appropriate. * liboctave/array/Range.cc(elem): Move function from Range.h Return base for first element of array. Return limit of range for end of array if appropriate. * liboctave/array/Range.cc(_rangeindex_helper, index): Return base for first element of array. Return limit of range for end of array if appropriate. * liboctave/array/Range.cc(min, max): Use '<=' or '>=' tests to return base or limit if appropriate. * liboctave/array/Range.cc(is_sorted): Place more common test first in if/else if/else tree. * liboctave/array/Range.cc(operator <<): Return base for first element of array. Return limit of range for end of array if appropriate. liboctave/array/Range.h(Range constructor): Put trivial 2-line constructor in .h file. liboctave/array/Range.h(elem): Move function which has become more complicated to Range.cc. * test/range.tst: Add %!tests for corner cases of base and limit of range.
author Rik <rik@octave.org>
date Fri, 01 Mar 2013 14:06:02 -0800
parents 8650eec57e9f
children 2a4f83826024
files libinterp/interpfcn/pr-output.cc liboctave/array/Range.cc liboctave/array/Range.h test/range.tst
diffstat 4 files changed, 137 insertions(+), 47 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/interpfcn/pr-output.cc	Fri Mar 01 12:46:56 2013 -0800
+++ b/libinterp/interpfcn/pr-output.cc	Fri Mar 01 14:06:02 2013 -0800
@@ -2724,14 +2724,17 @@
                 {
                   octave_quit ();
 
-                  double val = base + i * increment;
+                  double val;
+                  if (i == 0)
+                    val = base;
+                  else
+                    val = base + i * increment;
 
                   if (i == num_elem - 1)
                     {
                       // See the comments in Range::matrix_value.
-
-                      if ((increment > 0 && val > limit)
-                          || (increment < 0 && val < limit))
+                      if ((increment > 0 && val >= limit)
+                          || (increment < 0 && val <= limit))
                         val = limit;
                     }
 
--- a/liboctave/array/Range.cc	Fri Mar 01 12:46:56 2013 -0800
+++ b/liboctave/array/Range.cc	Fri Mar 01 14:06:02 2013 -0800
@@ -36,14 +36,6 @@
 #include "lo-utils.h"
 #include "Array-util.h"
 
-Range::Range (double b, double i, octave_idx_type n)
-  : rng_base (b), rng_limit (b + (n-1) * i), rng_inc (i),
-    rng_nelem (n), cache ()
-{
-  if (! xfinite (b) || ! xfinite (i))
-    rng_nelem = -2;
-}
-
 bool
 Range::all_elements_are_ints (void) const
 {
@@ -64,17 +56,24 @@
       cache.resize (1, rng_nelem);
       double b = rng_base;
       double increment = rng_inc;
-      for (octave_idx_type i = 0; i < rng_nelem; i++)
-        cache(i) = b + i * increment;
+      if (rng_nelem > 0)
+        {
+          // The first element must always be *exactly* the base.
+          // E.g, -0 would otherwise become +0 in the loop (-0 + 0*increment).
+          cache(0) = b; 
+          for (octave_idx_type i = 1; i < rng_nelem; i++)
+            cache(i) = b + i * increment;
+        }
 
       // On some machines (x86 with extended precision floating point
       // arithmetic, for example) it is possible that we can overshoot
       // the limit by approximately the machine precision even though
       // we were very careful in our calculation of the number of
-      // elements.
+      // elements.  The tests need equality (>= rng_limit or <= rng_limit)
+      // to have expressions like -5:1:-0 result in a -0 endpoint.
 
-      if ((rng_inc > 0 && cache(rng_nelem-1) > rng_limit)
-          || (rng_inc < 0 && cache(rng_nelem-1) < rng_limit))
+      if ((rng_inc > 0 && cache(rng_nelem-1) >= rng_limit)
+          || (rng_inc < 0 && cache(rng_nelem-1) <= rng_limit))
         cache(rng_nelem-1) = rng_limit;
     }
 
@@ -87,16 +86,68 @@
   if (i < 0 || i >= rng_nelem)
     gripe_index_out_of_range (1, 1, i+1, rng_nelem);
 
-  return rng_base + rng_inc * i;
+  if (i == 0)
+    return rng_base;
+  else if (i < rng_nelem - 1) 
+    return rng_base + i * rng_inc;
+  else
+    {
+      double end = rng_base + i * rng_inc;
+      if ((rng_inc > 0 && end >= rng_limit)
+          || (rng_inc < 0 && end <= rng_limit))
+        return rng_limit;
+      else
+        return end;
+    }
 }
 
+double
+Range::elem (octave_idx_type i) const
+{
+#if defined (BOUNDS_CHECKING)
+  return checkelem (i);
+#else
+  if (i == 0)
+    return rng_base;
+  else if (i < rng_nelem - 1) 
+    return rng_base + i * rng_inc;
+  else
+    {
+      double end = rng_base + i * rng_inc;
+      if ((rng_inc > 0 && end >= rng_limit)
+          || (rng_inc < 0 && end <= rng_limit))
+        return rng_limit;
+      else
+        return end;
+    }
+#endif
+}
+
+// Pseudo-class used for idx_vector.loop () function call
 struct _rangeidx_helper
 {
-  double *array, base, inc;
-  _rangeidx_helper (double *a, double b, double i)
-    : array (a), base (b), inc (i) { }
+  double *array, base, inc, limit;
+  octave_idx_type nmax;
+
+  _rangeidx_helper (double *a, double b, double i, double l, octave_idx_type n)
+    : array (a), base (b), inc (i), limit (l), nmax (n-1) { }
+
   void operator () (octave_idx_type i)
-    { *array++ = base + i * inc; }
+    {
+      if (i == 0)
+        *array++ = base;
+      else if (i < nmax) 
+        *array++ = base + i * inc;
+      else
+        {
+          double end = base + i * inc;
+          if ((inc > 0 && end >= limit)
+              || (inc < 0 && end <= limit))
+            *array++ = limit;
+          else
+            *array++ = end;
+        }
+    }
 };
 
 Array<double>
@@ -119,13 +170,14 @@
       octave_idx_type il = i.length (n);
 
       // taken from Array.cc.
-
       if (n != 1 && rd.is_vector ())
         rd = dim_vector (1, il);
 
       retval.clear (rd);
 
-      i.loop (n, _rangeidx_helper (retval.fortran_vec (), rng_base, rng_inc));
+      // idx_vector loop across all values in i, executing _rangeidx_helper (i) foreach i
+      i.loop (n, _rangeidx_helper (retval.fortran_vec (),
+                                   rng_base, rng_inc, rng_limit, rng_nelem));
     }
 
   return retval;
@@ -146,8 +198,7 @@
           retval = rng_base + (rng_nelem - 1) * rng_inc;
 
           // See the note in the matrix_value method above.
-
-          if (retval < rng_limit)
+          if (retval <= rng_limit)
             retval = rng_limit;
         }
 
@@ -166,8 +217,7 @@
           retval = rng_base + (rng_nelem - 1) * rng_inc;
 
           // See the note in the matrix_value method above.
-
-          if (retval > rng_limit)
+          if (retval >= rng_limit)
             retval = rng_limit;
         }
       else
@@ -268,7 +318,7 @@
   if (dim == 1)
     {
       if (mode == ASCENDING)
-          retval.sort_internal (sidx, true);
+        retval.sort_internal (sidx, true);
       else if (mode == DESCENDING)
         retval.sort_internal (sidx, false);
     }
@@ -281,10 +331,10 @@
 sortmode
 Range::is_sorted (sortmode mode) const
 {
-  if (rng_nelem > 1 && rng_inc < 0)
+  if (rng_nelem > 1 && rng_inc > 0)
+    mode = (mode == DESCENDING) ? UNSORTED : ASCENDING;
+  else if (rng_nelem > 1 && rng_inc < 0)
     mode = (mode == ASCENDING) ? UNSORTED : DESCENDING;
-  else if (rng_nelem > 1 && rng_inc > 0)
-    mode = (mode == DESCENDING) ? UNSORTED : ASCENDING;
   else
     mode = mode ? mode : ASCENDING;
 
@@ -298,12 +348,15 @@
   double increment = a.inc ();
   octave_idx_type num_elem = a.nelem ();
 
-  for (octave_idx_type i = 0; i < num_elem-1; i++)
-    os << b + i * increment << " ";
+  if (num_elem > 1)
+    {
+      // First element must be the base *exactly* (-0).
+      os << b << " ";
+      for (octave_idx_type i = 1; i < num_elem-1; i++)
+        os << b + i * increment << " ";
+    }
 
-  // Prevent overshoot.  See comment in the matrix_value method
-  // above.
-
+  // Prevent overshoot.  See comment in the matrix_value method above.
   os << (increment > 0 ? a.max () : a.min ()) << "\n";
 
   return os;
@@ -502,7 +555,7 @@
             n_elt++;
         }
 
-      retval = (n_elt >= std::numeric_limits<octave_idx_type>::max () - 1) ? -1 : n_elt;
+      retval = (n_elt < std::numeric_limits<octave_idx_type>::max () - 1) ? n_elt : -1;
     }
 
   return retval;
--- a/liboctave/array/Range.h	Fri Mar 01 12:46:56 2013 -0800
+++ b/liboctave/array/Range.h	Fri Mar 01 14:06:02 2013 -0800
@@ -50,7 +50,13 @@
       rng_nelem (nelem_internal ()), cache () { }
 
   // For operators' usage (to preserve element count).
-  Range (double b, double i, octave_idx_type n);
+  Range (double b, double i, octave_idx_type n)
+    : rng_base (b), rng_limit (b + (n-1) * i), rng_inc (i),
+      rng_nelem (n), cache ()
+    {
+      if (! xfinite (b) || ! xfinite (i))
+        rng_nelem = -2;
+    }
 
   double base (void) const { return rng_base; }
   double limit (void) const { return rng_limit; }
@@ -80,14 +86,7 @@
 
   double checkelem (octave_idx_type i) const;
 
-  double elem (octave_idx_type i) const
-    {
-#if defined (BOUNDS_CHECKING)
-      return checkelem (i);
-#else
-      return rng_base + rng_inc * i;
-#endif
-    }
+  double elem (octave_idx_type i) const;
 
   Array<double> index (const idx_vector& i) const;
 
--- a/test/range.tst	Fri Mar 01 12:46:56 2013 -0800
+++ b/test/range.tst	Fri Mar 01 14:06:02 2013 -0800
@@ -72,3 +72,38 @@
 %!assert ([ r ; uint32(z)          ], uint32(expect))
 %!assert ([ r ; uint64(z)          ], uint64(expect))
 
+## Test corner cases of ranges (base and limit)
+
+%!shared r, rrev, rneg
+%! r = -0:3;
+%! rrev = 3:-1:-0;
+%! rneg = -3:-0;
+
+%!assert (full (r), [-0 1 2 3])
+%!assert (signbit (full (r)), logical ([1 0 0 0]))
+%!assert (r(1), -0)
+%!assert (signbit (r(1)), true)
+%!assert (signbit (r(1:2)), logical ([1 0]))
+%!assert (signbit (r(2:-1:1)), logical ([0 1]))
+%!assert (signbit (r([2 1 1 3])), logical ([0 1 1 0]))
+
+%!assert (full (rrev), [3 2 1 -0])
+%!assert (signbit (full (rrev)), logical ([0 0 0 1]))
+%!assert (rrev(4), -0)
+%!assert (signbit (rrev(4)), true)
+%!assert (signbit (rrev(3:4)), logical ([0 1]))
+%!assert (signbit (rrev(4:-1:3)), logical ([1 0]))
+%!assert (signbit (rrev([1 4 4 2])), logical ([0 1 1 0]))
+
+%!assert (min (r), -0)
+%!assert (signbit (min (r)), true)
+%!assert (min (rrev), -0)
+%!assert (signbit (min (rrev)), true)
+
+%!assert (max (rneg), -0)
+%!assert (signbit (min (rneg)), true)
+
+%!assert (sort (r, "descend"), [3 2 1 -0])
+%!assert (signbit (sort (r, "descend")), logical ([0 0 0 1]))
+%!assert (signbit (sort (rrev, "ascend")), logical ([1 0 0 0]))
+