bugGNU Octave - Bugs: bug #31287, Certain assignments of empty...

 
 

bug #31287: Certain assignments of empty arrays give errors (Matlab incompatibility)

Submitter:  None
Submitted:  Sat 09 Oct 2010 10:04:08 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Iain Murray Originator Email:  -email is unavailable-
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

Thu 05 May 2016 11:23:33 AM UTC, comment #25: 

As the test cases in my original post now all work in the Hg tip version of Octave, I'm happy to see this bug marked as fixed. Thanks very much!

Iain Murray <imurray>
Sat 30 Apr 2016 10:09:36 AM UTC, comment #24: 

Running Olaf's script file #22338 from comment #10 reports no errors except "should not succeed but did" (many times).

Many of these use "false" in place of [], as JWE explicitly chose in comment #16, though not all.

I'm inclined to close this as "fixed", unless we particularly want Octave to throw more errors than now.

Status: need info -> fixed.

Lachlan Andrew <lachlan>
Sat 12 Dec 2015 08:31:04 PM UTC, comment #23: 

poking around old bugs.  just to reply to Robert's comment #22, this behavior is compatible with Matlab 2015a behavior.  (colon makes T 0x1, + gives nonconformant argument error)

Nicholas Jankowski <nrjank>
Group Member
Sat 15 Feb 2014 10:31:43 PM UTC, comment #22: 

The colon operator changes the size of an empty matrix:

octave:6> T=[]
T = [](0x0)
octave:7> T=T(:)
T = [](0x1)

This causes problems like:
octave:8> U=[]
U = [](0x0)
octave:9> T+U
error: operator +: nonconformant arguments (op1 is 0x1, op2 is 0x0)


Robert Jenssen <morgawr>
Tue 15 Oct 2013 04:53:11 PM UTC, comment #21: 

Iain, I didn't claim that the whole issue had been solved.  I only looked at the assignment of literal [] to delete elements.

I'm not completely opposed to going for compatibility here.  I can see your point about what happens when values happen to be vectors instead of matrices with dimensions > 1.

But I would like to try to understand precisely what the rules are first, not just try to fix this by allowing all assignments of empty arrays to succeed.  I don't think Matlab even allows that.  But what are the rules supposed to be?  Did Olaf get them right, or is there some simpler single rule that can explain the behavior (similar to what I think I found for the [] assignment case -- early exit from a loop)?

John W. Eaton <jwe>
Group administrator
Tue 15 Oct 2013 12:13:01 PM UTC, comment #20: 

Thank you for looking at this issue (and as always, for Octave). As the original submitter I don't think that the bug I actually submitted has been fixed though.

The first example from the original report still crashes in Octave tip. As that original post explains, this case can arrive from fairly reasonable code. The example was a simplified version of some real Matlab code that I was trying out in Octave.

I appreciate the idea that sizes should usually match. But it's always been the case in the Octave and Matlab languages that when dimensions are sometimes (but not always) of size 1, weird stuff can happen. Matlab and Octave have a ball of hacks to make these cases more friendly. Please have another look at the motivating example and see what you think.

Iain Murray <imurray>
Sat 12 Oct 2013 05:50:10 PM UTC, comment #19: 

It's OK with me.

I think the only remaining question is whether we should require empty matrix assignments to conform.  We do that for things like arithmetic ops so it seems to me we should also require it for assignment.

John W. Eaton <jwe>
Group administrator
Sat 12 Oct 2013 05:39:42 PM UTC, comment #18: 

Is it okay to mark this bug complete on the 3.8 bug fix list?

Rik <rik5>
Group administrator
Sat 12 Oct 2013 05:17:54 AM UTC, comment #17: 

As for the other cases, I'm not sure that we should allow them.  Shouldn't we require that the dimensions of the RHS of the assignment match the sizes of the indices even when the RHS is empty?  Not requiring that the dimensions match seems sloppy to me.

John W. Eaton <jwe>
Group administrator
Sat 12 Oct 2013 05:15:07 AM UTC, comment #16: 

I checked in the following change to fix the null assignment cases.

http://hg.savannah.gnu.org/hgweb/octave/rev/7ed397c8ca68

Note that my change accepts both [] and false equivalently to select 0 elements.  Matlab seems inconsistent here, for example, accepting x(false,false)=[] if x is 2d but not x(false,false,false)=[] if x is 3d.  I opted for what seems like better consistency to me.


John W. Eaton <jwe>
Group administrator
Wed 09 May 2012 01:48:15 PM UTC, comment #15: 

This looks like a regression since 3.0, and it was supposed to be fixed for 3.4, and it's still not fixed in 3.6.1.

Marking this as a regression.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Thu 13 Jan 2011 07:50:23 AM UTC, comment #14: 


> It would be interesting to me to see what Octave produces for
> all these test cases just for the index expression on the LHS.
> Does the dimension of that result match the dimension of the
> value on the RHS?


Octave already conforms to "rule 2.2", i.e. the dimensions of the indexed expression on the LHS and the dimensions of the RHS do match for successful empty assignment, with the "exception" that dimenensions being 1 can additionally be present at non-matching positions and numbers.

But for matrices ("rule 2.1"), Matlab allows any assignment as long as indexed LHS and RHS are both empty, but Octave does not.

Olaf Till <i7tiol>
Group Member
Wed 12 Jan 2011 07:43:53 PM UTC, comment #13: 

Are you sure the rules have to be this complex?

For example, for this case


x = ones (2, 2, 2)
x(false, []) = []


if we look at what the result is just for the index expression on the LHS, we would get [](0x0).  I think this is allowed because any time a variable is indexed, the extra dimensions are combined until there are as many dimensions as indices.  So in the above, it is as if we are really indexing a 2x4 array, not a 2x2x2 array.  Would it help if we first just applied that rule to the LHS and then proceeded as we do now?  Or are we already doing that?

It would be interesting to me to see what Octave produces for all these test cases just for the index expression on the LHS.  Does the dimension of that result match the dimension of the value on the RHS?

If so, then it would seem to me that we are not applying the same rules to indexing expressions when they appear on the LHS of an assignment as we are when they are not part of an assignment expression.

If not, are they at least both empty?

Maybe we simply need to apply the same rules for indexing the LHS as we do for indexing expressions when they are not part of an assignment expression before testing to see whether the assignment conforms?

John W. Eaton <jwe>
Group administrator
Wed 12 Jan 2011 04:58:40 PM UTC, comment #12: 

Btw, changing priority to High because John said he'd like this fixed before 3.4. release.

Jaroslav Hajek <highegg>
Wed 12 Jan 2011 04:57:45 PM UTC, comment #11: 

OK, bravo, you've carried it much farther than I expected. Well done :)

So I guess we all see that these rules are stupidly complex and probably most of them result from ad hoc implementation decisions. Since I think people mostly complain when some expression works in Matlab but fails in Octave, I'd say let's pick a reasonably simple ruleset generalizing these, i.e. let's be incompatible on the side of allowing more than Matlab.

Options:
A. Allow any empty assignment.
 - Simple, but maybe too simple.
B. Allow any empty null assignment, and for non-null assignments implement the rules 2.1 and 2.2 observed by Olaf.
C. Like B, but follow 1.1, 1.2 and 1.3 for null assignments, simplifying 1.3 to "logical array with all false elements".

My personal preference is B. Opinions?

Jaroslav Hajek <highegg>
Mon 03 Jan 2011 03:26:18 PM UTC, comment #10: 

Here is a corrected version of the deduced rules and an
attached testscript. The testscript ran without unforeseen
messages in Matlab 2009b, but I suggest that someone
verifies this, if possible with a more recent Matlab
version.


Rules for empty assignment, deduced from Matlab version
2009b

Empty assignment here means an assignment:

lhs(idx) = rhs;

where 'lhs' is a variable, 'idx' are indices separated by
commas, and 'rhs' is some expression, and at least on
dimension of 'lhs(idx)' is zero and at least one dimension
of 'rhs' is zero.

'lhs(idx)' will be called 'i_lhs' in the following.

In the following, refering to dimensions means the
dimensions as returned by 'size()'.

Without explicitly mentioning it, all the following (only)
applies to empty assignment.

1. So called null-assignment, i.e. rhs is the constant []
   (no variable).

   Succeeds if idx contains at most 1 non-'special' index or
   the second non-'special' index is preceded (at any
   position before it) by an index being an empty numeric
   array (see NOTEs below). 'Special' indices are:

1.1. a colon ':',

1.2. any empty numeric array,

1.3. a logical index of any length with all elements false
     (called just 'logical index' in the following); but
     with a logical index, there are the following
     restrictions (see NOTEs below):

1.3.1. If lhs is more than 2-dimensional, and idx contains
       more than 1 index, an index being a logical index
       gives an error if no further index being an empty
       numerical array is given.

1.3.2. idx can contain more than 1 logical index only if it
       contains less than 3 indices or it contains at least
       1 further index being an empty numerical array at
       position 1 or 2.

1.3.3. If idx contains a logical index as well as a colon :,
       assignment only succeeds if idx contains less than 3
       indices or idx contains at least 1 further index
       being an empty numeric array.

2. No null-assignment.

2.1. lhs <= 2-dimensional AND idx contains at most 2 indices
     AND rhs <= 2-dimensional

     Empty assignment always succeeds.

2.2. Cases not covered by 2.1.

     Empty assignment succeeds if and only if all dimensions
     != 1 (i.e. 0 or >= 2) are in the same order in i_lhs
     and rhs (which implies that there is the same number of
     these non-singleton dimensions in i_lhs and rhs).

     I.e. dimensions of 1 can be in arbitrary numbers and
     positions in i_lhs and rhs.

NOTE to rule 1 in general: Since more than 1 non-'special'
index is allowed if the second is preceded by an index being
an empty numeric array, I'd say more than 1 non-'special'
index should be always allowed in empty assignment. So the
the testscript will mark messages due to more than 1
non-'special' index (not preceded by an index being an empty
array) as 'DISPUTABLE'.

NOTE to rule 1.3: I'm not sure that this rule is consistent
with all cases. Even if it should be, if no less bizarre
rule can be found, I'd suggest not to copy this behavior,
but to leave off the restrictions 1.3.1 to 1.3.3.. The
attached testscript will print a message marked 'DISPUTABLE'
if only the restrictions 1.3.1 -- 1.3.3 are violated.

Olaf


(file #22338)

Olaf Till <i7tiol>
Group Member
Wed 22 Dec 2010 01:59:28 PM UTC, comment #9: 

Rules deduced for Matlab 2009b

In the following "dimensions" means dimensions as returned by size().

1. Special case "null assignment" (= [], literally)

The rule for the left hand side is given by the error message "A null assignment can have only one non-colon index."

The following, and only the following, qualify for a "colon index" in the sense of this rule:

- a colon ":",
- any numeric array with at least one dimension empty,
- only if the lhs is at most 2-dimensional: a logical index of arbitrary length with all elements false.

2. The other empty assignments

Definitions for assignments:
lhs(idx) = rhs;
ilhs means lhs(idx)

Matlab seems to treat 2 cases differently

2.1. First case: lhs <= 2-dimensional AND ilhs <= 2-dimensional AND rhs <= 2-dimensional

Empty assignment succeeds if and only if (any dimension in ilhs is 0 AND any dimension in rhs is 0).

2.2. Second case: all other empty assignments

Empty assignment succeeds if and only if all dimensions != 1 (i.e. 0 or >= 2) are in the same order in ilhs and rhs (which implies that there is the same number of these non-singleton dimensions in ilhs and rhs).

I.e. dimensions of 1 can be in arbitrary numbers and positions in ilhs and rhs.

End of rules.

I can make a test script, but not before January. I would like to have these rules reviewed and tested before (though of corse I will also test them with the script). Should I post them to a list for a wider audience of potential testers?

Olaf Till <i7tiol>
Group Member
Sun 12 Dec 2010 09:52:37 PM UTC, comment #8: 

I can easily run things in Matlab if somebody actually produces a script that tests the corner cases of the assignment stuff. The script would, however, have to be rigorous enough to be able to figure out the underlying system.

Søren

Søren Hauberg <hauberg>
Sun 12 Dec 2010 08:37:38 PM UTC, comment #7: 

I don't think increasing priority will help much here. As I said, we need someone with access to Matlab to come to the rescue.
FWIW, I think that the indexed assignment behavior in 3.0.1 was even far less compatible with Matlab than the current behavior, just the problems were elsewhere.

Jaroslav Hajek <highegg>
Fri 10 Dec 2010 07:54:51 PM UTC, comment #6: 

OK, fair enough. It sounds like there were significant improvements at the expense of a little collateral damage as opposed to a refactoring gone wrong.

Would it make sense for the priority to be bumped due to the fact that this bug was caused by those changes?

jkirby <jkirby>
Fri 10 Dec 2010 06:31:31 PM UTC, comment #5: 

Yes, I understand that it was working as you expect in a 3.0.x.

However, indexed assignment in Octave has been completely reworked since then.  For the most part the rewrite cleaned up and simplified many complicated special cases with much simpler rules that are faster and generally provide better compatibility.  What Jaroslav is saying is that we don't want to go back to the previous jumble of ad-hoc special cases.  So if this is to be fixed, we need to have some kind of general rule that can be applied.  If you want to see this fixed, then help us understand what the underlying rule is.  It would be helpful to provide test cases as well and show us what the results are in Octave, as he suggested.

John W. Eaton <jwe>
Group administrator
Fri 10 Dec 2010 06:06:17 PM UTC, comment #4: 

Just a FYI: the original submission examples for this does work in Octave 3.0.1 (windows) but not in 3.2.2 (as reported) or later. The duplicate bug (https://savannah.gnu.org/bugs/index.php?31764) that was only reported for logical indexing concurred with a subset of this breakage for around the same version numbers.

So at least in version 3.0.1 it was all working as expected.


jkirby <jkirby>
Fri 10 Dec 2010 04:44:23 PM UTC, comment #3: 

There is also a duplicate report here:

https://savannah.gnu.org/bugs/index.php?31764

John W. Eaton <jwe>
Group administrator
Fri 10 Dec 2010 04:39:58 PM UTC, comment #2: 

This is a repost of the comments Jaroslav made on Nov 25, and which were lost during the savannah outage:

I'm OK to fix this, but I have adopted the stance that I want to derive a general rule first. Fixing incompatibilities on an ad hoc basis is useless. I suppose the rule may be something like "A(I,J,...) = B always works if B is 0x0 and at least one of I,J,... is empty."

So, if you want to fix it, find a general rule like this (you can try this one, if it works) and then provide a test script, as detailed as possible, that works in Matlab some way (e.g. success/errors + different results if applicable) and should work in Octave the same way.

This has to be done by someone with Matlab (which I'm not) and is actually the bulk of the work, fixing the Octave code is then easy.  I'll be glad to do the latter part.

regards

John W. Eaton <jwe>
Group administrator
Sun 24 Oct 2010 04:58:08 PM UTC, comment #1: 

This is confirmed on a recent Hg tip (2010/10/24).  I find the motivation to correct reasonable, but this is not my area of expertise in the Octave code base.

Rik <rik5>
Group administrator
Sat 09 Oct 2010 10:04:08 PM UTC, original submission:  

The following minimal example gives "error: A(I,J,...) = X: dimensions mismatch" in Octave, while Matlab leaves Y unchanged without complaint:

Y = 1;
Y(1, zeros(1,0)) = zeros(0,0);

You might reasonably think that the above code is silly, so let me give a motivating example for altering Octave's behavior. Below is some code to replace NaNs with some default values. It works in Octave and Matlab:

A = [  1   2   3   4   5;
     1.4 2.3 4.5 2.3 3.2];
defaults = [2.1 3.1 4.1 5.1 6.1];
mask = isnan(A(2,:));
A(2,mask) = defaults(mask);

If a function has code like the above and is sometimes given arrays with one column, it breaks in Octave (but not in Matlab):

A = [  1;
     2.1];
defaults = [1.4];
mask = isnan(A(2,:));
A(2,mask) = defaults(mask);

The problem is that now size(defaults(mask)) is [0,0], so it looks like the minimal example and Octave says "error: A(I,J,...) = X: dimensions mismatch".

One could argue that the code should have used:
    A(2,mask) = defaults(1,mask);
Which does work in Matlab and Octave regardless of the number of columns in A. But making
    A(2,logical(0)) = zeros(0,0)
work would make more Matlab code work in Octave out-of-the-box. Matlab coders wouldn't usually put in the "1," when indexing a vector, and even Octave coders might not notice that an edge case is broken.


More examples:

% GROUP 1: for reference, these all work (do nothing) in both Matlab and Octave:
a = 1;
a(1, []) = zeros(1,0)
a(1, zeros(1,0)) = zeros(1,0)
a(1, logical(0)) = zeros(1,0)

% GROUP 2: These work (do nothing) in Matlab but currently give
% "error: A(I,J,...) = X: dimensions mismatch" in Octave
% Making Octave accept these would be good for compatibility.
a = 1;
a(1, []) = zeros(0,0)
a(1, zeros(1,0)) = zeros(0,0)
a(1, logical(0)) = zeros(0,0)

% GROUP 3: Like Group 2 these work in Matlab but not in Octave.
% The error message is different:
% "error: A null assignment can only have one non-colon index."
% which makes sense because assigning '[]' has special meaning.
% Although Matlab accepts all of the following, I can't think of a convincing
% use case. Maybe there is no point in "fixing" Octave for group 3(?).
a = 1;
a(1, []) = []
a(1, zeros(1,0)) = []
a(1, logical(0)) = []

The above experience was on:
GNU Octave, version 3.2.2
As shipped by Ubuntu 9.10 "karmic", Linx 2.6.31-22-generic i686
(I also briefly tried the Mercurial dev version)

I could probably be encouraged to try to come up with a patch. I thought I'd see if there was interest in changing Octave before spending too much time looking at the source. Also maybe one of the developers will know how to tweak this much faster than I would be able to.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #22338:  testscript_bug_31287.m added by i7tiol (4KiB - text/x-objcsrc)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by mtmiller (Updated the item)
  • -email is unavailable- added by morgawr (Posted a comment)
  • -email is unavailable- added by imurray (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by i7tiol (Posted a comment)
  • -email is unavailable- added by hauberg (Posted a comment)
  • -email is unavailable- added by highegg (Posted a comment)
  • -email is unavailable- added by jkirby (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by None (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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-05-05 lachlan Open/ClosedOpen Closed
    2016-04-30 lachlan StatusNeed Info Fixed
    2015-03-14 mtmiller Priority7 - High 5 - Normal
        Release3.2.2 dev
        Operating SystemGNU/Linux Any
    2014-02-15 rik5 Carbon-CopyRemoved 72865 -
    2013-10-17 rik5 Dependencies- bugs #35921 is dependent
    2012-05-09 jordigh Item GroupIncorrect Result Regression
        Assigned tohighegg None
    2011-01-12 highegg Priority3 - Low 7 - High
    2011-01-12 highegg Assigned toNone highegg
    2011-01-03 i7tiol Attached File- Added testscript_bug_31287.m, #22338
    2010-12-12 highegg StatusConfirmed Need Info
    2010-10-24 rik5 Priority5 - Normal 3 - Low
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code