bugGNU Octave - Bugs: bug #50776, Octave inconsistent with Matlab...

 
 

bug #50776: Octave inconsistent with Matlab for corner indexing case

Submitted by:  Richard <crobar>
Submitted on:  Mon 10 Apr 2017 10:12:45 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Richard CrozierOpen/Closed: Closed
Release: devOperating 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 Jump to the original submission)

Wed 12 Apr 2017 11:56:54 PM UTC, comment #12:

I added an xelem function to dim_vector for fast access to the internals (http://hg.savannah.gnu.org/hgweb/octave/rev/77a7f2fecd74).

I'm going to close this bug report, as all issues have been addressed.

Rik <rik5>
Project Administrator
Wed 12 Apr 2017 06:17:02 PM UTC, comment #11:

Rik: Yeah, Jaroslav loved the cute stuff.

And adding xelem to dim_vector would be fine with me.

John W. Eaton <jwe>
Project Administrator
Wed 12 Apr 2017 05:02:08 PM UTC, comment #10:

Very cute code:

As a side note, should idx_vector have an xelem method in analogy to the xelem method for Array?

Right now I see only elem(). For example, your new code is

But since the loop explicitly runs over the dimensions of the idx_vector there is no need to also run through an assert statement to check that.

Rik <rik5>
Project Administrator
Wed 12 Apr 2017 04:39:24 PM UTC, comment #9:

I didn't change anything about the way index values are stored, just what happens with the dimensions. Now we handle indices as "vectors" if they have all but one dimension equal to 1.

Yes, logical indices can be stored as vectors but only if the number of true values is small. See the idx_vector::idx_vector (const Array<bool>& bnda) constructor in idx-vector.cc.

John W. Eaton <jwe>
Project Administrator
Wed 12 Apr 2017 04:21:36 PM UTC, comment #8:

The patch works for me with the reporter's original test!

I wrote "any logical array index is first converted to a column vector" because in my terminology an array is an N-d object which is not a scalar or a vector, what we would traditionally just call a matrix. Any logical array which is not 1x1, 1xN, or Nx1 is first reshaped to Nx1 before indexing.

In pseudo-code

One of the advantages to logical indexing is that it takes less memory. Is that advantage still preserved? I think it is, but seems worth asking.

For example, for this test case

As you would expect, numeric indices use a lot more memory because they store everything as 8-byte doubles.

Is there some point where we convert the logical index to a linear index deep in the C++ code? That would be a bummer.

Rik <rik5>
Project Administrator
Wed 12 Apr 2017 02:24:04 PM UTC, comment #7:

I pushed the following changeset.

http://hg.savannah.gnu.org/hgweb/octave/rev/f1ea44e92df8

John W. Eaton <jwe>
Project Administrator
Wed 12 Apr 2017 01:03:07 PM UTC, comment #6:

Thanks for the pointer to the blog post. It doesn't seem to mention anything about N-d "vectors".

And yes, this may be inconsistent with the isvector function, but I still think we should probably be consistent with Matlab here.

"This is really a consequence of the fact that any logical array index is first converted to a column vector. And then the general rule is that for vector/vector operations it is the indexed object whose size is retained. "

Shouldn't that be "converted to a column vector if it is not already a vector"? Otherwise, wouldn't the result of array(logical_row) be a column vector? I think it ends up being a row.

John W. Eaton <jwe>
Project Administrator
Wed 12 Apr 2017 07:38:18 AM UTC, comment #5:

oh, and that

Richard <crobar>
Wed 12 Apr 2017 07:32:11 AM UTC, comment #4:

I can confirm JWE's test script produces no output in Matlab R2016b

Richard <crobar>
Tue 11 Apr 2017 11:45:54 PM UTC, comment #3:

See this article: http://blogs.mathworks.com/loren/2006/08/09/essence-of-indexing/

For logical indexing, an array index is always reshaped to a column vector before indexing.

Thus,

This is the generic case of a matrix indexed by a vector so it takes the index's shape.

In your table, a logical index always produces the shape of the indexed object for a vector.

This is really a consequence of the fact that any logical array index is first converted to a column vector. And then the general rule is that for vector/vector operations it is the indexed object whose size is retained.

I still think Octave is actually more correct because the text example is an array with three dimensions, not a vector, so it should be an example of matrix/vector indexing. But, we get to code around their quirks.

Rik <rik5>
Project Administrator
Tue 11 Apr 2017 10:39:48 PM UTC, comment #2:

I think there are some differences between logical and numeric indexing when only one index supplied.

I came up with the following rules:

I'm attaching a test script and associated function that can be used to verify these assumptions. If correct it should produce no output. Could someone test this in a recent Matlab and verify that it is correct? It fails a number of cases in Octave currently.

I'm also attaching a proposed patch for Octave to fix these problems.

Note that I had to add data members to idx_vector to keep track of the real original index dimensions and whether it is a logical index. In spite of the fact that there is an orig_dims data member there, it seems like it can be something other than the original dimensions of the index. Also, there's a lot of complication there with transforming one type of index to another (logical to vector, for example) so I'm not sure whether my change is actually sufficient since the notion of "logical index" is not transferred if the internal index representation changes. It would be great to simplify and clean up this mess but I can't work on it more at the moment.

(file #40367, file #40368, file #40369)

John W. Eaton <jwe>
Project Administrator
Tue 11 Apr 2017 07:29:39 PM UTC, comment #1:

The issue is not so much logical/linear indexing, rather it is the determination of what the indexed object is.

For indexing, where

As you can see, there is only one odd case which is a vector indexed by a vector. Octave treats the 3-D example matrix as a matrix, which means the resulting shape is the shape of the indexing object (a vector).

Notice how the output follows the orientation of the input as it should. On the other hand, Matlab seems to be treating the original matrix as a vector and thus retaining the shape of the original object. What does this return in Matlab?

My guess is that they are internally inconsistent and they don't think 'x' is a vector, except for the odd case of indexing.

Rik <rik5>
Project Administrator
Mon 10 Apr 2017 10:12:45 AM UTC, original submission:

Octave does not use the same rules for doing logical indexing on vectors as Matlab in terms of the shape of the output. In Matlab the rule seems to be: for vectors, return always the same shape of vector being indexed, for anything else return a column vector

For example, in Matlab:

In Octave:

This is using a dev version of Octave, but from quite a while ago, but I've even deleted the repo clone I built it from, so I can't give hg id:

Richard <crobar>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #40367:  idxtest.m added by jwe (640B - text/x-csrc)
file #40368:  vecidxtest2.m added by jwe (7KiB - text/x-csrc)
file #40369:  idx-diffs.txt added by jwe (8KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Updated the item)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by crobar (Submitted the item)
  • -unavailable- added by crobar
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 12 Apr 2017 11:56:54 PM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Wed 12 Apr 2017 02:24:04 PM UTCjweStatusPatch Submitted=>Ready For Test
    Tue 11 Apr 2017 10:39:48 PM UTCjweAttached File-=>Added idxtest.m, #40367
      Attached File-=>Added vecidxtest2.m, #40368
      Attached File-=>Added idx-diffs.txt, #40369
      StatusConfirmed=>Patch Submitted
    Tue 11 Apr 2017 07:29:39 PM UTCrik5StatusNone=>Confirmed
      SummaryOctave logical indexing inconsistent with Matlab=>Octave inconsistent with Matlab for corner indexing case
    Mon 10 Apr 2017 10:12:45 AM UTCcrobarCarbon-Copy-=>Added crobar

    Back to the top


    Powered by Savane 3.1-cleanup1