changeset 30853:fc3bd70cd1be stable

eliminate range<T>::make_constant function Overloading ranges as a way to create constant arrays adds complexity to the range class for limited benefit. It is currently only used for double-valued ranges and only provides constant row vectors. If this optimization is determined to be useful, it would probably be best to provide a data type that allows N-d constant arrays. The legacy range class was revived in a previous changeset so that constant ranges (increment == 0) can be loaded from existing data files and converted to ordinary arrays automatically. * Range.h (range<T>::make_constant): Delete. Remove all uses.
author John W. Eaton <jwe@octave.org>
date Tue, 22 Mar 2022 00:01:53 -0400
parents 95725e6ad9c1
children eba0a86471b9 aaf689533e7b
files libinterp/corefcn/data.cc libinterp/octave-value/ov-range.cc liboctave/array/Range.h
diffstat 3 files changed, 9 insertions(+), 68 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/data.cc	Mon Mar 21 23:58:35 2022 -0400
+++ b/libinterp/corefcn/data.cc	Tue Mar 22 00:01:53 2022 -0400
@@ -4179,14 +4179,6 @@
     case oct_data_conv::dt_double:
       if (iscomplex)
         retval = ComplexNDArray (dims, Complex (val, 0));
-      else if (dims.ndims () == 2 && dims(0) == 1)
-        {
-          // FIXME: If this optimization provides a significant
-          // benefit, then maybe there should be a special storage
-          // type for constant value arrays.
-          double dval = static_cast<double> (val);
-          retval = range<double>::make_constant (dval, dims(1));
-        }
       else
         retval = NDArray (dims, val);
       break;
@@ -4290,11 +4282,6 @@
     case oct_data_conv::dt_double:
       if (iscomplex)
         retval = ComplexNDArray (dims, Complex (val, 0));
-      else if (dims.ndims () == 2 && dims(0) == 1 && math::isfinite (val))
-        // FIXME: If this optimization provides a significant benefit,
-        // then maybe there should be a special storage type for
-        // constant value arrays.
-        retval = range<double>::make_constant (val, dims(1));
       else
         retval = NDArray (dims, val);
       break;
@@ -4359,13 +4346,7 @@
       break;
 
     case oct_data_conv::dt_double:
-      if (dims.ndims () == 2 && dims(0) == 1 && math::isfinite (val))
-        // FIXME: If this optimization provides a significant benefit,
-        // then maybe there should be a special storage type for
-        // constant value arrays.
-        retval = range<double>::make_constant (val, dims(1));
-      else
-        retval = NDArray (dims, val);
+      retval = NDArray (dims, val);
       break;
 
     default:
--- a/libinterp/octave-value/ov-range.cc	Mon Mar 21 23:58:35 2022 -0400
+++ b/libinterp/octave-value/ov-range.cc	Tue Mar 22 00:01:53 2022 -0400
@@ -766,13 +766,7 @@
   if (! is)
     error ("load: failed to load range constant");
 
-  if (inc != T (0))
-    r = octave::range<T> (base, inc, limit, rev);
-  else
-    {
-      octave_idx_type numel = static_cast<octave_idx_type> (limit);
-      r = octave::range<T>::make_constant (base, numel, rev);
-    }
+  r = octave::range<T> (base, inc, limit, rev);
 
   return true;
 }
@@ -946,13 +940,8 @@
       if (swap)
         swap_bytes<sizeof (bool)> (&rev);
     }
-  if (inc != T (0))
-    r = octave::range<T> (bas, inc, lim, rev);
-  else
-    {
-      octave_idx_type numel = static_cast<octave_idx_type> (lim);
-      r = octave::range<T>::make_constant (bas, numel, rev);
-    }
+
+  r = octave::range<T> (bas, inc, lim, rev);
 
   return true;
 }
@@ -1280,13 +1269,7 @@
 
       bool rev = with_reverse ? static_cast<bool> (rangevals[3]) : false;
 
-      if (rangevals[2] != T (0))
-        r = octave::range<T> (rangevals[0], rangevals[2], rangevals[1], rev);
-      else
-        {
-          octave_idx_type numel = static_cast<octave_idx_type> (rangevals[1]);
-          r = octave::range<T>::make_constant (rangevals[0], numel, rev);
-        }
+      r = octave::range<T> (rangevals[0], rangevals[2], rangevals[1], rev);
     }
 
   H5Tclose (range_type);
--- a/liboctave/array/Range.h	Mon Mar 21 23:58:35 2022 -0400
+++ b/liboctave/array/Range.h	Tue Mar 22 00:01:53 2022 -0400
@@ -76,14 +76,11 @@
     }
 
     // Allow conversion from (presumably) properly constructed Range
-    // objects and to create constant ranges (see the static
-    // make_constant method).  The values of base, limit, increment,
-    // and numel must be consistent.
+    // objects.  The values of base, limit, increment, and numel must be
+    // consistent.
 
     // FIXME: Actually check that base, limit, increment, and numel are
-    // consistent.
-
-    // FIXME: Is there a way to limit this to T == double?
+    // consistent?
 
     range (const T& base, const T& increment, const T& limit,
            octave_idx_type numel, bool reverse = false)
@@ -91,26 +88,6 @@
         m_final (limit), m_numel (numel), m_reverse (reverse)
     { }
 
-    range (const T& base, const T& increment, const T& limit,
-           const T& final, octave_idx_type numel, bool reverse = false)
-      : m_base (base), m_increment (increment), m_limit (limit),
-        m_final (final), m_numel (numel), m_reverse (reverse)
-    { }
-
-    // We don't use a constructor for this because it will conflict with
-    // range<T> (base, limit) when T is octave_idx_type.
-
-    static range<T> make_constant (const T& base, octave_idx_type numel,
-                                   bool reverse = false)
-    {
-      // We could just make this constructor public, but it allows
-      // inconsistent ranges to be constructed.  And it is probably much
-      // clearer to see "make_constant" instead of puzzling over the
-      // purpose of this strange constructor form.
-
-      return range<T> (base, T (), base, numel, reverse);
-    }
-
     // We don't use a constructor for this because it will conflict with
     // range<T> (base, limit, increment) when T is octave_idx_type.
 
@@ -120,7 +97,7 @@
     {
       // We could just make this constructor public, but it allows
       // inconsistent ranges to be constructed.  And it is probably much
-      // clearer to see "make_constant" instead of puzzling over the
+      // clearer to see "make_n_element_range" instead of puzzling over the
       // purpose of this strange constructor form.
 
       T final_val = (reverse ? base - (numel - 1) * increment