bugGNU Octave - Bugs: bug #46385, Can't Repmat Structure Arrays

 
 

bug #46385: Can't Repmat Structure Arrays

Submitted by:  Max Görner <maxg>
Submitted on:  Fri 06 Nov 2015 09:38:43 AM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Other
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating 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 Jump to the original submission)

Sat 05 Aug 2017 04:18:42 AM UTC, comment #21:

Reshape has been fixed in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/3544f88a2bb5). The original motivating code now passes. Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Sat 10 Sep 2016 03:57:34 PM UTC, comment #20:

A quick search turns up this bug from 2014 where reshape produces the same error for some struct arrays:
https://savannah.gnu.org/bugs/?43650

my guess is if we fixed that than this bug would be resolved as well.

Nicholas Jankowski <nrjank>
Sat 10 Sep 2016 02:31:38 PM UTC, comment #19:

1 The key error appears to be in handling structs with reshape. Sincevtats a c function I can't step through it, but maybe a separate bug should be opened. The failing reshape command works fine in MATLAB.

2 The repelem workaround does appear to solve this bug. Should that patch be added as a dependency in this report, then when that gets pushed this could too?

3 Would someone be able to take a quick peek at the repelem patch and and comment if there's something glaring that needs to be done on it?

Nicholas Jankowski <nrjank>
Sat 10 Sep 2016 11:42:49 AM UTC, comment #18:

No, repelem hasn't been included yet, but it still could be.

John W. Eaton <jwe>
Project Administrator
Sat 10 Sep 2016 11:37:14 AM UTC, comment #17:

Repelem was a missing function. Patch submitted under bug #45497. Pushing that patch should make this patch work. Did the repelem patch make it into 4.2?

http://savannah.gnu.org/bugs/?45497

Nicholas Jankowski <nrjank>
Fri 09 Sep 2016 08:45:26 PM UTC, comment #16:

The first error was that 'repelem' does not exist. I tried changing that to 'repelems', but still got a failure.

Rik <rik5>
Project Administrator
Fri 09 Sep 2016 08:22:02 PM UTC, comment #15:

Rik, curious what error you receive. I applied Markus's patch (calling repelem if reshape is given a struct) and I get no errors and the output matches Matlab.

Nicholas Jankowski <nrjank>
Tue 06 Sep 2016 10:36:33 PM UTC, comment #14:

The patch from bug #48368 has been committed to the repository. But that is still not enough to fix this problem. I tried the patch attached to this report and still get an error. I'm changing the status back to Confirmed from Patch Submitted since the patch doesn't appear to work.

Rik <rik5>
Project Administrator
Fri 01 Jul 2016 11:38:02 PM UTC, comment #13:

Nice thought, Nicholas, but even with the patch from bug #48368 the result is

Lachlan Andrew <lachlan>
Project Member
Fri 01 Jul 2016 07:57:56 PM UTC, comment #12:

yes, in Octave Rik's script produces:

as it seems to be appending the first element (4) from s(2).a

It would be interesting to see if fixing size also fixes repmat without Markus's patch. My initial guess is calling repelem from within repmat would not be quite as efficient if it wasn't needed.

Nicholas Jankowski <nrjank>
Fri 01 Jul 2016 07:43:14 PM UTC, comment #11:

Since I'm not setup to recompile, this is using the 4.0.2 release.

Confirming Rik's test for Matlab:

I switched to zeros to verify that it was picking up the 0 from the next 'a' and not from 'b' or elsewhere.

Nicholas Jankowski <nrjank>
Fri 01 Jul 2016 07:07:31 PM UTC, comment #10:

Oops. Our posts crossed, Rik.

(Also, I notice that the answer to my question to Nicholas is that he was using ones(100), not zeros (100), which is why he got size(M.a)=1; size just uses the first value in the second argument.)

I have a simple patch for size, but I'll file a separate bug report for it.

Lachlan Andrew <lachlan>
Project Member
Fri 01 Jul 2016 06:47:24 PM UTC, comment #9:

Nicholas, it looks like the size(M.a) is treating M.a as a cs-list.

Out of interest, this is what I get for a recent Octave tip:

When you got size(bigM.a) = 100, was that with Markus's patch?

Lachlan Andrew <lachlan>
Project Member
Fri 01 Jul 2016 06:35:20 PM UTC, comment #8:

Actually, it sounds like Matlab is returning a comma-separated list, which is what is supposed to happen when you ask for the same field from a structure array.

A simple example for testing in Matlab:

Octave appears not to handle the 's.a' case correctly.

Rik <rik5>
Project Administrator
Fri 01 Jul 2016 05:56:24 PM UTC, comment #7:

apparently much of comments #4-6 can be disregarded. I had left M defined as the already repmat'd struct way back at the start for the Matlab case, making them look different. fixing that the outputs to almost all the code is identical.

BUT, a difference does exist between Octave and Matlab output with the error that Matlab produces for the one size call:

For both Octave and matlab:

In Octave:

In Matlab:

from this output it appears that size(bigM.a) is using something as a second size argument specifying one of the dimensions. Matlab yells that something improper is being passed to that size argument (likely the second 100x100 array). If we use zeros(100) instead of ones(100) to make M:

somewhat confirming that octave is taking the first element for size.

maybe size needs a varargin check to catch this?

Nicholas Jankowski <nrjank>
Fri 01 Jul 2016 03:00:01 PM UTC, comment #6:

looking through repmat, there are places where it makes decisions based on the result of size(A) or numel(A). For the example below octave gives numel(mtest) = 2, while Matlab gives numel(mtest) = 4.

if the patch using repelem works, I think it's just because it avoids those calls. When we rewrote repelem, I didn't do any tests on structs (or cells for that manner) so I don't at the moment know how it's handling them differently. But while this would make repmat work, there still appears to be something broken in how octave determines the size of struct arrays.

Nicholas Jankowski <nrjank>
Fri 01 Jul 2016 02:51:28 PM UTC, comment #5:

sorry for the syntax typo's in the last comment, but to clarify: it appears that the struct array created by repmat has the right pieces. a bit more, not using repmat:

Octave:

Matlab 2016a:

So, this isn't just a repmat issue...

Nicholas Jankowski <nrjank>
Fri 01 Jul 2016 02:41:16 PM UTC, comment #4:

messing with this a little, there's a discrepancy with how Octave determines the size of a struct array:

now with repmat:

Matlab 2016a reports it as:

looking at how the element sizes are reported

Octave:

Matlab 2016a:
+verbatim

>> size(bigM)

ans =
1 4

>> size(bigM.a)

Error using size
Too many input arguments.

>> size(bigM.b)

Error using size
Too many input arguments.

>> size(bigM(1).a)

ans =
100 100

>> size(bigM(2).a)

ans =
100 100

>> size(bigM(1).b)

ans =
1 1

>> size(bigM(2).b)

ans =
1 1
-verbatim-

so, Octave returns 1x2 as the answer, Matlab 1x4. Maybe the size issue is some of the cause of the problem. or is this a separate bug?

Nicholas Jankowski <nrjank>
Fri 27 Nov 2015 02:01:29 PM UTC, comment #3:

the problem is in reshape, which is called by repmat.

Octave failed here with a dimension mismatch across fields in struct.

Using repelem from #45497 makes it work without touching reshape.

(file #35577)

Markus Bergholz <markuman>
Fri 13 Nov 2015 08:05:31 PM UTC, comment #2:

It's a 1x4 struct array.

Max Görner <maxg>
Fri 13 Nov 2015 05:45:37 PM UTC, comment #1:

When you run the code in Matlab, what is the resulting output? Is it a 1x4 struct array or does it make something else like a 1x2 cell array where element 1 is the 1x2 struct array M and element 2 is the 1x2 struct array M?

Rik <rik5>
Project Administrator
Fri 06 Nov 2015 09:38:43 AM UTC, original submission:

I wanted to repmat structure arrays, but this will fail.

An example is:

Under Matlab R2015b this works like a charm. But even ignoring Matlab I think that code should work.

Max Görner <maxg>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #35577:  repmat.patch added by markuman (615B - text/x-diff)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by nrjank (Posted a comment)
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by markuman (Updated the item)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by maxg (Submitted the item)
  •  

    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 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 05 Aug 2017 04:18:42 AM UTCrik5StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Sat 05 Aug 2017 04:04:03 AM UTCrik5Dependencies-=>bugs #51634 is dependent
    Tue 06 Sep 2016 10:36:33 PM UTCrik5StatusPatch Submitted=>Confirmed
    Sat 11 Jun 2016 12:39:28 PM UTClachlanStatusConfirmed=>Patch Submitted
      Release4.0.0=>dev
      Operating SystemGNU/Linux=>Any
    Fri 27 Nov 2015 02:01:29 PM UTCmarkumanAttached File-=>Added repmat.patch, #35577
    Fri 13 Nov 2015 05:45:37 PM UTCrik5StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1