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

 
 

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

Submitter:  Michael Godfrey <godfrey>
Submitted:  Wed 27 Feb 2013 03:27:27 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Michael Godfrey Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 04 Mar 2013 12:57:14 AM UTC, comment #9: 

[Note type in "help typeinfo": "return an cell array".]

OK, I understand now.  min and max have to return a type double matrix and Octave is retaining the underlying dimensions of the operations even though they are empty.  Further examples:

octave:24> (1:1:3)'*[1 1]
ans =

   1   1
   2   2
   3   3

octave:25> (1:-1:3)'*[1 1]
ans = [](0x2)
octave:26> (1:-1:3)*[1 1]
error: operator *: nonconformant arguments (op1 is 1x0, op2 is 1x2)

octave:28> ones(0,0)
ans = [](0x0)
octave:29> []
ans = [](0x0)
octave:30> typeinfo(ones(0,0))
ans = matrix
octave:31> typeinfo([])
ans = null_matrix
octave:32>


Dan Sebald <sebald>
Mon 04 Mar 2013 12:25:41 AM UTC, comment #8: 

Range::min() and Range::max() are never called unless there is at least one element in the range. 

On the second question, when you create an empty range, such as 1:-1:2, you do not get a null range but actually a null matrix gets created.  You can check with typeinfo() and the return object is not "range" but "matrix".  The dimensions of the null matrix are 1x0 as one can get, for example, by "ones (1,0)".  Unless there is a compelling reason to have the dimensions be exactly 0x0 I would let sleeping dogs lie because I couldn't quickly figure out which code is generating this particular matrix.

Rik <rik5>
Group administrator
Sun 03 Mar 2013 05:13:08 AM UTC, comment #7: 

OK, after looking at the code, I've convinced myself that the same algorithm is being used in all places (aside from the one you mentioned that you think is cruft).  That's good.  Probably still some minor quibbles, but fine.

One last thing I'd like you to consider is the min/max results.  Why

191 double retval = 0.0;
192 if (rng_nelem > 0)

?  It is probably meaningless what the initialized return value is because when I run octave with something like

octave:1> min(1:-3)
ans = [](1x0)

the result is the empty set, just as with

octave:2> min([])
ans = [](0x0)

Obviously 0.0 is not the minimum displayed so somewhere else the empty set is overriding the 0.0.  So, one could probably put a more sensible

191 double retval = NAN;
192 if (rng_nelem > 0)

and have it work just as well.

Also notice in the two results above that the size of the empty array are slightly different.  Why is it that

octave:3> 1:-3
ans = [](1x0)

has a 1x0 as opposed to a 0x0?

Dan Sebald <sebald>
Sun 03 Mar 2013 02:21:22 AM UTC, comment #6: 

The situation isn't as bad as it appears.

For item #1, the print routine operates at human time scales.  The current code will generate results much faster than they can be consumed either by a pager (of order seconds until a user hits the space bar) or by a monitor (60 Hz). 

Special-casing the first element isn't attractive here because the body of the loop is so large.

For item #2, min/max routines use deduction to eliminate needless calculations.  Line 198 is correct because it is followed by lines 201/202.

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

The lines above are not duplicates.  "r(1)" tests that the elem() function call works.  "min (r)" verifies that the function min() works.  The second body of code in the min() routine is exercised by

%!assert (min (rrev), -0)

which is several lines down in range.tst.

The operator '<<' is apparently unused which is why I didn't bother completing this.  I need to ask jwe if this is truly unused and why we bother to keep it around if it is.

Rik <rik5>
Group administrator
Sat 02 Mar 2013 02:26:05 AM UTC, comment #5: 

And what about this hunk and the last element?  Is that b + i * increment still?  I'm not sure when this hunk would be called.


   2.188 +  if (num_elem > 1)
   2.189 +    {
   2.190 +      // First element must be the base exactly (-0).
   2.191 +      os << b << " ";
   2.192 +      for (octave_idx_type i = 1; i < num_elem-1; i++)
   2.193 +        os << b + i * increment << " ";
   2.194 +    }
   2.195

Dan Sebald <sebald>
Sat 02 Mar 2013 02:14:08 AM UTC, comment #4: 

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.

Dan Sebald <sebald>
Sat 02 Mar 2013 01:14:58 AM UTC, comment #3: 

Rik,

Your patch definitely corrects the problem
identified here.  Fixes the problem in
stemleaf, and is Matlab compatible as well!!!

Probably best to close this report.  If there
are other problems with range operator they
should be treated separately.

Thanks!

Michael Godfrey <godfrey>
Group Member
Thu 28 Feb 2013 05:47:16 AM UTC, comment #2: 

Well, that certainly is interesting.  There are several issues here.

First, the example illustrates that Matlab is retaining the sign bit when -0 is entered.

Second, the example illustrates that Matlab does display as an integer, not %4.0f as Octave is doing.

Third, the example is not observing the rule [a:z:b] -> a a+1z a+2z ... a+Lz.

Dan Sebald <sebald>
Thu 28 Feb 2013 04:57:22 AM UTC, comment #1: 

I sent the following to the maintainers list.
Hope it helps:

Would it be useful to move this discussion to bug #38423?
This report shows the case [-4:2-0] where Matlab produces
-4 -2 -0

but Octave produces;
-4 -2 0

This is an incompatibility with Matlab.

Michael

Michael Godfrey <godfrey>
Group Member
Wed 27 Feb 2013 03:27:27 AM UTC, original submission:  

Matlab gives:

>> zz=[-4:2:-0]


zz =

    -4    -2     0

>> fprintf('%g \n',zz)

-4
-2
-0

>>


But Ovtave gives:
octave:74> zz = [-4:2:-0]
zz =

  -4  -2   0

Note that Octave correctly handles:
octave:50> [-0:-2:-4]
ans =

  -0  -2  -4 

as does Matlab.

So, the sequence of the form [-n:-m:-0]
needs to be corrected.


Michael Godfrey <godfrey>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by godfrey (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-03-03 rik5 StatusNone Fixed
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code