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

 
 

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

Submitted by:  None
Submitted on:  Tue 21 Dec 2010 03:54:32 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: David Bateman <dbateman>
Originator Name: Originator Email: -unavailable-
Open/Closed: ClosedRelease: dev
Operating System: GNU/Linux

Add a New Comment (Rich MarkupRich 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)

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

Ok pushed and closed

D.

David Bateman <dbateman>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
Fri 24 Dec 2010 03:46:21 PM UTC, comment #1:

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

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

In Octave devel (3.3.54+), the following crashes:
returns:
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 File(s):
   
   
Comment:
   

Attached Files
file #22365:  changeset2.single added by dbateman (4kB - application/octet-stream)
file #22361:  changeset.single added by dbateman (2kB - 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
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by hauberg (Updated the item)
  • -unavailable- added by dbateman (Posted a comment)
  • -unavailable- added by dbateman
  • -unavailable- added by rik5 (Posted a comment)
  •  

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

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 09 Jan 2011 08:24:17 PM UTCdbatemanStatusNeed Info=>Fixed
      Open/ClosedOpen=>Closed
    Thu 06 Jan 2011 10:18:56 PM UTCdbatemanAttached File-=>Added changeset2.single, #22365
    Wed 05 Jan 2011 10:26:09 PM UTCdbatemanAttached File-=>Added changeset.single, #22361
    Wed 05 Jan 2011 09:07:52 AM UTChaubergAttached File-=>Added a2_ml2010a.mat, #22357
      Attached File-=>Added ab_ml2010a.mat, #22358
    Tue 04 Jan 2011 08:58:13 PM UTCdbatemanStatusFixed=>Need Info
    Tue 04 Jan 2011 06:17:05 PM UTCrik5Open/ClosedClosed=>Open
    Tue 28 Dec 2010 07:46:11 PM UTCdbatemanItem GroupCrash=>Incorrect Result
      StatusConfirmed=>Fixed
      Assigned toNone=>dbateman
      Open/ClosedOpen=>Closed
      Dependencies-=>Depends on bugs #30800
      Carbon-Copy-=>Added t_gdahl
    Fri 24 Dec 2010 03:46:21 PM UTCrik5StatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup