# HG changeset patch # User John W. Eaton # Date 1647921713 14400 # Node ID fc3bd70cd1bec23982300b1211f0b093b93b11e2 # Parent 95725e6ad9c12beb8eb3ce1234c2bf18c9dd1c9f eliminate range::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::make_constant): Delete. Remove all uses. diff -r 95725e6ad9c1 -r fc3bd70cd1be libinterp/corefcn/data.cc --- 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 (val); - retval = range::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::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::make_constant (val, dims(1)); - else - retval = NDArray (dims, val); + retval = NDArray (dims, val); break; default: diff -r 95725e6ad9c1 -r fc3bd70cd1be libinterp/octave-value/ov-range.cc --- 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 (base, inc, limit, rev); - else - { - octave_idx_type numel = static_cast (limit); - r = octave::range::make_constant (base, numel, rev); - } + r = octave::range (base, inc, limit, rev); return true; } @@ -946,13 +940,8 @@ if (swap) swap_bytes (&rev); } - if (inc != T (0)) - r = octave::range (bas, inc, lim, rev); - else - { - octave_idx_type numel = static_cast (lim); - r = octave::range::make_constant (bas, numel, rev); - } + + r = octave::range (bas, inc, lim, rev); return true; } @@ -1280,13 +1269,7 @@ bool rev = with_reverse ? static_cast (rangevals[3]) : false; - if (rangevals[2] != T (0)) - r = octave::range (rangevals[0], rangevals[2], rangevals[1], rev); - else - { - octave_idx_type numel = static_cast (rangevals[1]); - r = octave::range::make_constant (rangevals[0], numel, rev); - } + r = octave::range (rangevals[0], rangevals[2], rangevals[1], rev); } H5Tclose (range_type); diff -r 95725e6ad9c1 -r fc3bd70cd1be liboctave/array/Range.h --- 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 (base, limit) when T is octave_idx_type. - - static range 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 (base, T (), base, numel, reverse); - } - // We don't use a constructor for this because it will conflict with // range (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