bugGNU Octave - Bugs: bug #45219, Incorrect behavior with...

 
 

bug #45219: Incorrect behavior with broadcasting and mixed real/complex matrices

Submitted by:  Michael C. Grant <mcgrant>
Submitted on:  Sat 30 May 2015 07:10:20 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  In Progress Assigned to:  None
Originator Name:  Michael Grant Open/Closed:  Open
Release:  dev Operating System:  Any

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)

Mon 29 Feb 2016 03:11:44 AM UTC, comment #10: 

Thanks for this and the other commits/feedback, Rik.

I'll freshen the patch and look at the automatic broadcasting soon.

Lachlan Andrew <lachlan>
Mon 29 Feb 2016 02:36:22 AM UTC, comment #9: 

I applied the first of Lachlan's patches here http://hg.savannah.gnu.org/hgweb/octave/rev/a1aa52bff079.  This fixes the error originally seen with bsxfun.  There is still a problem with the operator '+' when it needs to do automatic broadcasting.

t1 = randn(6,3)+1j*randn(6,3);
t2 = randn(6,1);
s1 = sparse(t1);
s2 = sparse(t2);

bsxfun (@plus, s1, s2)  # This works NOW
s1 + s2
error: operator +: nonconformant arguments (op1 is 6x3, op2 is 6x1)

The second patch from Lachlan no longer applies cleanly.  I can look at it again when it is refreshed to current tip.

Rik <rik5>
Project Administrator
Mon 15 Feb 2016 08:53:04 PM UTC, comment #8: 

@devs: please review this patch.

@juanpi: I presume that waiting 5 months wouldn't consider to be impatient :-)

Avinoam Kalma <avinoam>
Project Member
Fri 25 Sep 2015 08:26:25 AM UTC, comment #7: 

Hi Thanks,

Please be patient, devs do care for patches but the manpower available is not a lot. If you think your patch has been sitting here for too long, just ping it and send an email to the mailing list. It probably it is just that dev haven0t seen it.

Juan Pablo Carbajal <juanpi>
Project Member
Fri 25 Sep 2015 06:57:32 AM UTC, comment #6: 

I should have mentioned that the second patch has two minor tweaks to the code as well as documentation.

The first makes the error message on the slow path the same as that on the fast path if the matrices have incompatible sizes.

The second is a simplification of the way of finding dvc(i), the size of the i'th dimension of the result.

Lachlan Andrew <lachlan>
Fri 25 Sep 2015 06:53:11 AM UTC, comment #5: 

This report actually describes something unrelated to the problems with automatic broadcasting of sparse matrices.  The bug with the explicit use of bsxfun is that the variable  C  in bsxfun() is not initialised properly for sparse matrices.  It is fixed by the attached patch.  It affects broadcast between any two sparse matrices of different storage types, such as single/double.

There is also a more subtle bug in bsxfun (f, a, b) when
f (a(:,1), b(:,1))  has a different type from  f (a(:,2), b(:,2)).
The slow-path code converts singles to doubles, instead of doubles to
singles.  To see this, try

octave> a = [1, 2; 1, 2]; b = [1, 1];
octave> C = { single(1) , 2 };
octave> f = @(a,b) ([C{a}]');
octave> t = bsxfun (f, a, b);
octave> tt = [f(a(1,1),b(1,1)), f(a(1,2),b(1,2))
              f(a(2,1),b(1,1)), f(a(2,2),b(1,2))]
octave> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        C           1x2                         12  cell
        a           2x2                         32  double
        b           1x2                         16  double
        f           1x1                          0  function_handle
        t           2x2                         32  double
        tt          2x2                         16  single

Total is 17 elements using 108 bytes

This is not too serious; Matlab gives an error if the output of  f  has a different storage type, except for promoting real to complex.  I'd be happy to fix this if it would be committed, but most of the patches I have submitted recently haven't been reviewed yet, so I'm not sure if it is worth it.

FWIW, the code in bsxfun() is pretty hard to follow, so I've written a bit of documentation, in the second attached patch.

(file #34976, file #34977)

Lachlan Andrew <lachlan>
Tue 02 Jun 2015 04:50:09 PM UTC, comment #4: 

Retagging release from 4.0.0-rc4 to 4.0.0.

John W. Eaton <jwe>
Project Administrator
Sat 30 May 2015 07:20:53 PM UTC, comment #3: 

Thanks for the report. This is mostly the same as previously reported bug #41441, can you add the specific details about complex types to that report?

Mike Miller <mtmiller>
Project Administrator
Sat 30 May 2015 07:18:01 PM UTC, comment #2: 

Further investigation reveals that the same issues occur with elementwise and broadcasted multiplication and division as well. The key combination is the mixture of real and complex sparse matrices.

Michael C. Grant <mcgrant>
Sat 30 May 2015 07:12:27 PM UTC, comment #1: 

Sorry, markup typo cut off the report. Here it is again.

t1 = randn(6,3)+1j*randn(6,3);
t2 = randn(6,1);
s1 = sparse(t1);
s2 = sparse(t2);

These work fine:

t1 + t2
bsxfun( @plus, t1, t2 )

These fail, each in different ways, but they should produce the same results as above, only sparse:

>> s1 + s2
error: operator +: nonconformant arguments (op1 is 6x3, op2 is 6x1)
>> bsxfun(@plus,s1,s2)
error: concatenation operator not implemented for '<unknown type>' by 'sparse complex matrix' operations

Matlab does the correct thing with the bsxfun functions, but of course it does not consider the bare additions to be correct.

Michael C. Grant <mcgrant>
Sat 30 May 2015 07:10:20 PM UTC, original submission:  

t1 = randn(6,3)+1j*randn(6,3);
t2 = randn(6,1);
s1 = sparse(t1);
s2 = sparse(t2);

These work fine:

Michael C. Grant <mcgrant>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #34976:  bug_45219.patch added by lachlan (515B - text/x-patch)
file #34977:  bsxfun_cc_comments.patch added by lachlan (9KiB - text/x-patch)

 

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 avinoam (Posted a comment)
  • -email is unavailable- added by juanpi (Posted a comment)
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by mcgrant (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-29 rik5 StatusNone => In Progress
        Release4.0.0 => dev
    2015-09-25 lachlan Attached File- => Added bug_45219.patch, #34976
        Attached File- => Added bsxfun_cc_comments.patch, #34977
    2015-06-02 jwe Release4.0.0-rc4 => 4.0.0

    Back to the top


    Powered by Savane 3.5