[Octave-bug-tracker] [bug #38423] [k:l:m] not compatible for m = -0

[Dan Sebald]

Follow-up Comment #4, bug #38423 (project octave):

Before closing, I'd like to make some comments.  Looks good for the most part,
but I wonder if some things could be marginally improved while having the code
fresh in your mind.

1))))) Right now the code is programmed to be code-efficient, as opposed to
CPU efficient.  That is, an alternate to the interior conditional would be to
repeat the relatively short interior code in a "triple loop".  E.g.:

> for (octave_idx_type i = col; i < lim; i++)
>   {
>     octave_quit ();
>
>     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))
>         val = limit;
>       }
>
>     os << " ";
>
>     pr_float (os, val, fw, scale);
>   }

First, other places this is written "if (i==0)..if (i < num_elem-1)..else".

Nonetheless, it is typically going to be the case that the array is long and
the "else" above is what happens most.  Unfortunately, I don't see any easy
way for programming for CPU efficiency because operations on the elements are
done by calling all these functions with an index value.  Oh well.

2))))) Are you sure that you caught everything?  I see there are some
simplified max/min routines that are short cutting a full array comparison by
using the extrema by deduction:

189 Range::min (void) const
190 {
191 double retval = 0.0;
192 if (rng_nelem > 0)
193 {
194 if (rng_inc > 0)
195 retval = rng_base;
196 else
197 {
198 retval = rng_base + (rng_nelem - 1) * rng_inc;
199
200 // See the note in the matrix_value method above.
201 if (retval <= rng_limit)
202 retval = rng_limit;
203 }
204
205 }
206 return retval;
207 }

If I understand correctly, this line is no longer consistent with the changes
you made:

198 retval = rng_base + (rng_nelem - 1) * rng_inc;

Also note that the tests you added to the code don't exercise that hunk of
code:

+%!assert (r(1), -0)
+%!assert (min (r), -0)

The above two lines of code are testing the exact same thing because r is an
array, not an expression.  The additional test instead of min(r) should
probably be

+%!assert (min (-3:0), -0)

The above might exercise the code hunk I listed above.  There is a similar
hunk for the max operation.

3))))) In the "help" documentation for the range operation, could you please
redo the text so that I accurately and unambiguously states how the range is
being created.  All the -0/first element/last element stuff could be real
confusing not to mention the array values themselves.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?38423>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/