bugGNU Octave - Bugs: bug #31942, Save/load single precision...

 
 

bug #31942: Save/load single precision variables with MATLAB data format fails

Submitter:  None
Submitted:  Tue 21 Dec 2010 03:54:32 PM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  dbateman
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * dev
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 09 Jan 2011 08:24:17 PM UTC, comment #11: 

Ok pushed and closed

D.

David Bateman <dbateman>
Group Member
Sun 09 Jan 2011 06:40:42 PM UTC, comment #10: 

David, all the examples listed in this report seem to work now, so I think you should go ahead and apply the patch and close this rerport.

If someone notices that there is still some problem, then please open a new bug report.

John W. Eaton <jwe>
Group administrator
Thu 06 Jan 2011 10:18:56 PM UTC, comment #9: 

I'm starting to remember why I hate the matlab file format....

The new issue has to do with the way matlab treats small (tag,value) pairs. That is a matlab data element is made up of a number of pairs (tag,value) where the tag contains the type of the value and its size in bytes. Typically the tag is 8 bytes with 4 bytes of the type followed by 4 bytes for the length of value.. and here's the catch. If the size saved in the tag is 4bytes of less, then the type is coded in 2 bytes, the length in two bytes and the data in four bytes!

As the length of a single element in the double type is 8 bytes they were never saved with short tags.. Single elements of the single type however are. So when calculating the length of the data to be saved we now need to take into account short tags for the single precision type.. My copy-paste to create the single-precision load/save code didn't take this into account.

I've attached a new changeset and hopefully this time there are no further issues. The new patch also avoids some copying when calculating the element length by casting the arrays as const.

Given the past history of this bug I won't commit this and close this bug till I have a report that there aren't any other issues.

D.

(file #22365)

David Bateman <dbateman>
Group Member
Thu 06 Jan 2011 04:54:27 PM UTC, comment #8: 

Unless I made a mistake when applying the patch, it is still not entirely fixed.
This now works:
  a=struct('field1',single(0:1),'field2',single(0));
  save a.mat a -v6; load a
But this doesn't work any more:
  a=struct('field1',single(0),'field2',single(0));
  save a.mat a -v6; load a

Thanks.

Anonymous
Wed 05 Jan 2011 10:26:09 PM UTC, comment #7: 

Ok, I see the problem.. There was no version of get_save_type function in ls-util.cc for float values and the data was saved as LS_DOUBLE rather than LS_FLOAT as it should have been. Adding a version of this function

extern save_type
get_save_type (float max_val, float min_val);

that returns LS_FLOAT fixes this problem for me. I should probably also replace the code

(nel + 1) & ~0x1LL

with something generic that handles both 32 and 64-bit versions of octave_idx_type cleanly as well and propose replacing this as well. I believe the changeset attached fully fixes this issue, but I prefer feedback before pushing it and closing this bug

D.



(file #22361)

David Bateman <dbateman>
Group Member
Wed 05 Jan 2011 01:07:59 PM UTC, comment #6: 

Thanks for this Søren.

Here is another example that crashes and does not need to have MATLAB at hand:

  clear a;
  a.field1 = single(1:2);
  a.field2 = single(0);
  save a.mat a -v6;
  load a.mat

Thanks.

Anonymous
Wed 05 Jan 2011 09:07:52 AM UTC, comment #5: 

I have done the following in Matlab 2010a:

>> a = single(pi);
>> b = single(exp(1));
>> save -v6 ab_ml2010a.mat a b
>> clear a
>> a.field1 = single(pi);
>> a.field2 = single(exp(1));
>> save -v6 a2_ml2010a.mat a


The resulting files are attached.

(file #22357, file #22358)

Søren Hauberg <hauberg>
Tue 04 Jan 2011 08:58:13 PM UTC, comment #4: 

As I said in the bug report and #30800, I no longer have access to a copy of matlab and so can't easily debug this.. My only solution would be to get you to upload the file saved by matlab and compare it against the same octave version byte by byte. Though I'd prefer a simpler file with no structures and one or two variables. Octave doesn't reduce floats to integers when saving them like matlab does (ie. single(1) will be save as a uint8 by matlab to save space). So does doing something like

a = single(pi);
b = single(exp(1));
save -v6 ab.mat a b

fail to  create a file loadable by matlab? If it does could you run the same code in matlab and upload the file ab.mat? If not could you try

clear a
a.field1 = single(pi);
a.field2 = single(exp(1));
save -v6 a.mat a

D.

David Bateman <dbateman>
Group Member
Tue 04 Jan 2011 01:39:36 PM UTC, comment #3: 

Hi,

It's definitely better but there is still something wrong when saving the file as MATLAB doesn't load it properly.
Try with the following:
clear a;a.field1=single(1);a.field2=single(2);save a.mat a -v6;load a.mat
If you load the the Octave-created 'a.mat' in MATLAB, you get this:
a =
    field1: []
    field2: []
while the other way round works ('a.mat' created by MATLAB and successfully read by Octave).

Many thanks.

Anonymous
Tue 28 Dec 2010 07:46:11 PM UTC, comment #2: 

There wasn't any saving of the single precision type in the matlab format till it was added after the bug report #30800 (see https://savannah.gnu.org/bugs/?30800). I made a slight error in this previous fix that prevented single precision arrays with an odd number of elements being saved. The size of the data to save needs to be an integer number of 8 bytes long with 4 bytes of padding if needed (and it was), but the length of the data element saved in the header of the element didn't take this into account.

The changeset

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

addresses this. As this fixes this bug report for me I'm closing it.

D.

PS. I added the reporter of bug #30800 in copy to this message so that he is aware of the issue.

David Bateman <dbateman>
Group Member
Fri 24 Dec 2010 03:46:21 PM UTC, comment #1: 

Confirmed on a recent tip (11412:a8a9f062d0ef) on 12/24/2010.

Rik <rik5>
Group administrator
Tue 21 Dec 2010 03:54:32 PM UTC, original submission:  

In Octave devel (3.3.54+), the following crashes:

clear a;
a.field=single(1);
save a.mat a -v6;
load a.mat

returns:

error: load: invalid element type = 0
error: load: trouble reading binary file `a.mat'

while it works fine with current stable version (3.2.4).

It seems to be due to a bug with the handling of single precision variables.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #22365:  changeset2.single added by dbateman (4KiB - application/octet-stream)
file #22361:  changeset.single added by dbateman (2KiB - application/octet-stream)
file #22357:  a2_ml2010a.mat added by hauberg (320B - application/octet-stream)
file #22358:  ab_ml2010a.mat added by hauberg (240B - application/octet-stream)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by hauberg (Updated the item)
  • -email is unavailable- added by dbateman (Posted a comment)
  • -email is unavailable- added by dbateman
  • -email is unavailable- added by rik5 (Posted a comment)
  •  

    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
    2011-01-09 dbateman StatusNeed Info Fixed
        Open/ClosedOpen Closed
    2011-01-06 dbateman Attached File- Added changeset2.single, #22365
    2011-01-05 dbateman Attached File- Added changeset.single, #22361
    2011-01-05 hauberg Attached File- Added a2_ml2010a.mat, #22357
        Attached File- Added ab_ml2010a.mat, #22358
    2011-01-04 dbateman StatusFixed Need Info
    2011-01-04 rik5 Open/ClosedClosed Open
    2010-12-28 dbateman Item GroupSegfault, Bus Error, etc. Incorrect Result
        StatusConfirmed Fixed
        Assigned toNone dbateman
        Open/ClosedOpen Closed
        Dependencies- Depends on bugs #30800
        Carbon-Copy- Added t_gdahl
    2010-12-24 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code