bugGNU Octave - Bugs: bug #63833, Matrix indexing is inconsistent

 
 

bug #63833: Matrix indexing is inconsistent

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Fri 24 Feb 2023 04:39:51 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Invalid / Not an Octave Bug Assigned to:  None
Originator Name:  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

Fri 24 Feb 2023 11:09:25 PM UTC, comment #10: 

As promised earlier, I pushed a doc patch to stable describing this more explicitly. https://hg.savannah.gnu.org/hgweb/octave/rev/0d77475a7507

Arun Giridhar <arungiridhar>
Group Member
Fri 24 Feb 2023 07:16:41 PM UTC, comment #9: 

@rik: Thanks. Have pushed such a change here https://hg.savannah.gnu.org/hgweb/octave/rev/a098cc74d9a5

It turned out that the edge case was very specific and could be detected by comparing the number of rows before and after the indexing.

Arun Giridhar <arungiridhar>
Group Member
Fri 24 Feb 2023 07:16:25 PM UTC, comment #8: 

agreed. permute and reshape before and after operating on dim1 are fairly lightweight compared to some of the other games that need to be played to operate on arbitrary dimensions.

Nicholas Jankowski <nrjank>
Group Member
Fri 24 Feb 2023 06:30:34 PM UTC, comment #7: 

You might time the for loop dec2hex.m which is 16 iterations versus the indexing solution.


+  ## Convert to char matrix and return.
+  ## We used to return this in a single line:
+  ##    hstr = ("0123456789ABCDEF")(d+1);
+  ## But there are edge cases governing the sizes of row and column vectors
+  ## that cause problems with output size, so we use a loop instead.
+  hstr = repmat (' ', size (d));
+  v = "0123456789ABCDEF";
+  for t = 0:15
+    hstr(d == t) = v(t + 1);
+  endfor


The time difference might be meaningless, or it might be worth addressing.  I did a lot of work on the set functions and what I found there was that it was easy to do some input checks to find out what the desired shape of the output would be, then change all inputs to a specific form such as column vectors, do the algorithm, then just change shape at the end.

Rik <rik5>
Group administrator
Fri 24 Feb 2023 06:18:21 PM UTC, comment #6: 

I'm late to this report, but I confirm everything that Nicholas said.  This is the wonderful world of Matlab compatibility and we just have to deal with it.  It is a pretty frequent occurrence that an m-file has an algorithm to calculate a result, and then the final step is simply to adjust type or dimensions for Matlab compatibility.  See the functions in the set/ directory like intersect.m which has at the end


    ## Adjust output orientation for Matlab compatibility
    if (isrowvec)
      c = c.';
    endif


Rik <rik5>
Group administrator
Fri 24 Feb 2023 06:05:28 PM UTC, comment #5: 

yes, in almost all of the functions I've written/edited, at some point I've had to add a hack to handle orientation of outputs for vectors. less than ideal, but it is predictable.

closing report.

Nicholas Jankowski <nrjank>
Group Member
Fri 24 Feb 2023 05:19:21 PM UTC, comment #4: 

@nrjank: Thank you! That was helpful context.

I'll add that to our documentation as well.

I noticed this behavior when I was getting weird bugs in testing dec2hex as part of bug #63282. In particular, dec2hex(0:15) was returning everything on one line instead of 16 separate lines but fixing that with a transpose was causing dec2base(1234) to return on different lines instead of one line. Since we can't change indexing behavior for Matlab compatibility, I'll work around it with a loop in that function.

Arun Giridhar <arungiridhar>
Group Member
Fri 24 Feb 2023 05:16:25 PM UTC, comment #3: 

with that you'll see a number of places in the code where to expressions like B = A(c)(:).' are used to ensure a row vector.

Nicholas Jankowski <nrjank>
Group Member
Fri 24 Feb 2023 05:13:38 PM UTC, comment #2: 

The behavior is defined at:

https://www.mathworks.com/help/matlab/ref/subsref.html?s_tid=doc_ta#mw_d8b30de1-3cbd-48ed-b8e6-4cb29fbef460


A(I) is an array formed from the elements of A specified by the subscript vector I. The resulting array is the same size as I except for the special case where A and I are both vectors. In this case, A(I) has the same number of elements as I but has the orientation of A.


Nicholas Jankowski <nrjank>
Group Member
Fri 24 Feb 2023 05:03:23 PM UTC, comment #1: 

the short answer is that yes, all of what's below is matlab compatible.  would have to dig a bit more to see if there's sensible logic at play or just "we're compatible"

Nicholas Jankowski <nrjank>
Group Member
Fri 24 Feb 2023 04:39:51 PM UTC, original submission:  

A 2x3 index matrix gives a 2x3 result and a 3x2 index matrix gives a 3x2 result, as expected:

>> x = (1:3);

>> x([2 3 1; 3 1 2])
ans =

   2   3   1
   3   1   2

>> x([2 3 1; 3 1 2]')
ans =

   2   3
   3   1
   1   2


But if the index matrix is a vector, then the result is always a row vector, never a column vector.

>> x([2 3 1])
ans =

   2   3   1

>> x([2 3 1]')
ans =

   2   3   1


Transposing the original matrix (not the index matrix) causes the result to also always be transposed:

>> y = x';

>> y([2 3 1])
ans =

   2
   3
   1

>> y([2 3 1]')
ans =

   2
   3
   1


But if the original matrix is bigger than just a single row or column, then the indexing does behave as expected.

octave:7> A = [1 2; 3 4]
A =

   1   2
   3   4

octave:8> A([1 2])
ans =

   1   3

octave:9> A([1 2]')
ans =

   1
   3


Unless this inconsistent behavior is done to satisfy some weird Matlab quirk, it feels like a bug. What is the context for this?

Documentation page doesn't mention this behavior: https://docs.octave.org/latest/Index-Expressions.html

Arun Giridhar <arungiridhar>
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 nrjank (Posted a comment)
  • -email is unavailable- added by arungiridhar (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
    2023-02-24 nrjank StatusNone Invalid / Not an Octave Bug
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code