bugGNU Octave - Bugs: bug #55622, for loop does not run over a...

 
 

bug #55622: for loop does not run over a matrix with colums but no row

Submitted by:  Marco Caliari <caliari>
Submitted on:  Fri 01 Feb 2019 03:02:30 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Patch Submitted Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  GNU/Linux

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Sat 16 Feb 2019 11:16:21 AM UTC, comment #17: 

Or what if the compatible behavior is prohibited in the --safe mode?

Anonymous
Sat 16 Feb 2019 09:09:28 AM UTC, comment #16: 

What do you think if we leave the compatible behavior in the --traditional mode?

Marco Caliari <caliari>
Project Member
Fri 15 Feb 2019 10:24:12 PM UTC, comment #15: 

Quoting from the Matlab documentation on empty arrays

An empty array in MATLAB is an array with at least one dimension length equal to zero. Empty arrays are useful for representing the concept of "nothing" programmatically. For example, suppose you want to find all elements of a vector that are less than 0, but there are none. The find function returns an empty vector of indices, indicating that it couldn't find any elements less than 0.

A = [1 2 3 4];
ind = find(A<0)

ind =

  1x0 empty double row vector

Many algorithms contain function calls that can return empty arrays. It is often useful to allow empty arrays to flow through these algorithms as function arguments instead of handling them as a special case. If you do need to customize empty array handling, you can check for them using the isempty function.

The use of empty arrays, per se, is fine.  As The Mathworks point out, it is often useful to be able to write an algorithm without having to special case whether there will be any elements to act on.

As an example, consider transforming between 1-based indexing (which Octave uses) to 0-based indexing (which the C language uses).  This is pretty easy, you just need to subtract 1.

A = [1 2 3 4];
ind = find (A)
ind =

   1   2   3   4

new_ind = ind - 1
new_ind =

   0   1   2   3

And this algorithm still works well if we put in a condition

A = [1 2 3 4];
ind = find (A > 2)
ind =

   3   4

new_ind = ind - 1
new_ind =

   2   3

And finally, it still works when there are no elements

A = [1 2 3 4];
ind = find (A > 4)
ind = [](1x0)
new_ind = ind - 1
new_ind = [](1x0)

I don't see any problem with allowing empty matrices to participate in mathematical operations.  However, confusing code-dependent issues arise if you say that looping over empty matrices is permissible.

For example, a common coding strategy might be to calculate some collection of elements and then loop over them like so.

for elem = collection
  ...
endfor

For the command find(), Matlab returns the empty dimension in the second dimension (columns).  But there is nothing that guarantees that an empty matrix has 0 columns.  User code that defines the variable collection might choose the opposite convention and create 0xN style empty matrices in which case the for loop is operating N times.

Or something innocuous like a transpose could upset things.

Say I have an algorithm for which I want to operate on the rows of the matrix.  Since the for loop returns columns, I need to transpose the matrix first.

x = [1, 2; 3, 4];
for elem = x'
  elem
end
elem =

   1
   2

elem =

   3
   4

So far that's fine.  But if x is an empty matrix then the control becomes confusingly caught up with the decision to act on rows.  In the case of find() the 1x0 result becomes 0x1 and the loop is executed once rather than zero times.  It's also data dependent.

x = [1 2 3 4];
idx = find (x > 4)
idx = [](1x0)
idx = find (x' > 4)
idx = [](0x1)

Lastly, this is very likely to lead to syntax errors.  Most of the time, a programmer is going to want to do something with the loop element from the collection and this will require an indexing operation that fails because the element is empty.

Last example,

x = [1 2 3 4]';
idx = find (x > 4)
for var = idx
  var(1) = var(1) + 1;  % increment first element in variable
end

Under Matlab this errors out with "Index exceeds the number of array elements (0)".  Currently, Octave has no problem with this code because it doesn't execute the body of the for loop when the right-hand side is empty.

Rik <rik5>
Project Administrator
Thu 14 Feb 2019 08:12:38 PM UTC, comment #14: 

It seems that MATLAB has used the concept of nullable type.
From the documentation:

"An empty array in MATLAB is an array with at least one dimension length equal
to zero. Empty arrays are useful for representing the concept of "nothing" programmatically."

All nonabstract MATLAB classes have a method named empty. For example we can write:

A = int16.empty(5,0);

It means that all non abstract types all nullable. Therefore we can make array of nulls and iterate over nulls.
--Interesting discussion--

Anonymous
Thu 14 Feb 2019 05:14:03 PM UTC, comment #13: 

I still disagree, and think we should leave Octave's current behavior as is.

In terms of UX/UI design I prefer to have the minimum number of principles and be able to to derive the behavior from these, rather than having lots of ad hoc exceptions.

For example, from the Declaration of Independence, "All men are created equal".  But, starting in 1896 (Plessy vs. Ferguson and doctrine of "separate but equal"), you had to special case the general principle based on location.  If you were in the North or West of the U.S. the general principle applied, but if you were in the South then you used the exception.  This turned out to be a dumb idea and the U.S eventually went back to just one guiding principle in 1954 (Brown vs. Board of Education of Topeka).

My conception of a for loop over an object (as opposed to a range like 1:1000) is that it is equivalent to a foreach loop over the elements of the object.  Hence, if the object is empty, there should be no looping.  For me, looping over empty matrices means creating a mental exception only for "for" loops which seems unneccessary.

Of course, Matlab, like Moses, has said, "looping occurs over the number of columns".  But, I guess I'm skeptical of the stone tablets.  We don't always follow Matlab when they veer off the rails which I think is the case here.

As a practical matter, I think user's code would be improved by choosing the loop variable more explicitly, rather than relying on this questionable behavior.  If you want to look a certain number of times, do that.

%% Clear code
for i = 1:5
  disp ("Hello World");
end

%% Unclear code
idx = zeros (0,5);
for i = idx
  disp ("Hello World");
end

Also as a practical matter, I don't think this odd construct occurs with much frequency so changing the few instances that occur wouldn't be burdensome.

With all that said, this isn't an issue I'm particularly interested in arguing over.  If you need to commit the patch for some clients who require 100% compatibility then it happens.

Rik <rik5>
Project Administrator
Mon 11 Feb 2019 04:17:38 PM UTC, comment #12: 

I disagree.

The for loop in Matlab is documented to loop over the columns of a matrix.  When indexing an N-d array as a matrix, Matlab joins the columns from all dimensions as if they were one.  Octave also does this re-dimensioning when evaluating an N-d array in a for loop command.  The behavior is only different when the leading dimension is the only zero dimension.

For example, if you have an array like x = ones (2,3,4), you can write x(:,1), x(:,2), ..., x(:,12) to index all the columns.  This behavior is consistent for zero-sized arrays.  If you have x = ones (0,3,4), you can perform the same indexing.  Only when one or more of the dimensions beyond the first is zero do you have no columns to index in the re-dimensioned array.  By not looping when only the first dimension equal to zero, we are less compatible with Matlab and inconsistent with other array indexing.

We've already copied the weirder part of for N-d array for loops, which is to always assign the re-dimensioned value to the loop index even when there are zero trips through the loop.

I believe the attached change makes the behavior compatible.

(file #46234)

John W. Eaton <jwe>
Project Administrator
Mon 11 Feb 2019 07:17:01 AM UTC, comment #11: 

I agree with the earlier decision and upholding it, closing.

Mike Miller <mtmiller>
Project Administrator
Mon 11 Feb 2019 01:03:51 AM UTC, comment #10: 

We decided not to implement this questionable behavior back in 2017.  Is there new evidence that would make us want to reverse that decision?  This is very much a corner case, and very odd.  My vote is to leave the decision as is and close this report.

Rik <rik5>
Project Administrator
Sat 02 Feb 2019 07:32:02 PM UTC, comment #9: 

Again, that looks like an artifact of implementation.  How would one guess that in loops, N-D arrays are squeezed to 2-D before using as the looping structure.  All very unintuitive, hence a bad UX/UI, even if an underlying principle can be found.

Rik <rik5>
Project Administrator
Sat 02 Feb 2019 01:39:59 PM UTC, comment #8: 

Rik:  It is consistent behavior if you first reshape the N-d array in

for i = ndarray
  ...
end

into a matrix using ndarray(:,:).

John W. Eaton <jwe>
Project Administrator
Fri 01 Feb 2019 09:23:39 PM UTC, comment #7: 

@jwe: Emphasizing the irrationality of the behavior, the example you gave executes 0 times rather than 3.

Rik <rik5>
Project Administrator
Fri 01 Feb 2019 09:15:39 PM UTC, comment #6: 

I had forgotten about the previous bug report but just found it now.  There, I wrote, "These seem almost like some kind of accident of implementation rather than conscious decisions about how things should work. But who knows, right? Is this Matlab behavior documented anywhere?"  I suppose it is documented, in that for loops are documented to iterate over the columns of a matrix.

BTW, what does the following do in Matlab?

for i = zeros (0, 3, 0)
  i
end

Does it loop three times or not at all?

John W. Eaton <jwe>
Project Administrator
Fri 01 Feb 2019 07:36:12 PM UTC, comment #5: 

@Mike: good digging.  I'm inclined not to emulate Matlab.  Adding jwe to the CC list.

Rik <rik5>
Project Administrator
Fri 01 Feb 2019 06:42:12 PM UTC, comment #4: 

For reference, this was last discussed in bug #50893. At that time, it was decided to be intentionally incompatible, the evaluator was changed to not loop over any empty array expression (https://hg.savannah.gnu.org/hgweb/octave/rev/fd7a16594614).

Mike Miller <mtmiller>
Project Administrator
Fri 01 Feb 2019 06:34:04 PM UTC, comment #3: 

I think it's debatable how useful this is.  It seems to be an artifact of the implementation: Matlab loops over the number of columns regardless of whether the actual expression is empty. 

For reference, this loop executes three times in Matlab

for i = zeros (0,3), disp ('hey'), end

Rik <rik5>
Project Administrator
Fri 01 Feb 2019 03:59:43 PM UTC, comment #2: 

@Kai: I discussed it with jwe on IRC, I think he is fixing it.

Marco Caliari <caliari>
Project Member
Fri 01 Feb 2019 03:14:58 PM UTC, comment #1: 

Evil statement ;-)  And it loops even in Matlab R2018b, one can see 'hey' once there, while Octave does not loop.

Kai Torben Ohlhus <siko1056>
Project Member
Fri 01 Feb 2019 03:02:30 PM UTC, original submission:  

The following code

for i = (3:2)', disp ('hey'), end

does not loop, while it should since (3:2)' has one column. It used to loop at least up to 4.2.2.

Marco Caliari <caliari>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46234:  loop-iteration-diffs.txt added by jwe (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by caliari (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-11 jwe Attached File- => Added loop-iteration-diffs.txt, #46234
        StatusWont Fix => Patch Submitted
        Open/ClosedClosed => Open
    2019-02-11 mtmiller StatusConfirmed => Wont Fix
        Open/ClosedOpen => Closed
    2019-02-01 rik5 Carbon-Copy- => Added jwe
    2019-02-01 siko1056 StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.5