Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#22 closed defect (fixed)

Grass r.out.mat 64bit Matlab reading problem

Reported by: alexice Owned by: grass-dev@…
Priority: minor Milestone: 6.3.0
Component: Default Version: 6.2.3
Keywords: r.out.mat 64bit Cc:
CPU: Unspecified Platform: Unspecified

Description

Grass raster maps which are exported with r.out.mat in an 64bit environment can not be read into Matlab (2007a). If I transfer the grass database to my 32bit laptop and export it there, it works and can be read by the same Matlab version.

If any more information is needed, please contact me.

Cheers, Alexice

Attachments (1)

roads_64bit.mat.gz (18.8 KB ) - added by neteler 16 years ago.
Spearfish roads raste map, exported on 64bit Linux box (Mandriva 2008.0)

Download all attachments as: .zip

Change History (15)

comment:1 by hamish, 16 years ago

Hi Alexice,

I am not too surprised at hearing that.

raster/r.out.mat/main.c: The mrows, ncols, format_block, and realflag variables are type "long" and written with fwrite(...,sizeof(long),...);. They should be written as 4 bytes on all platforms. The name_len variable too.

I think it's an easy fix but I have only passing exposure to 64bit programming. I don't have a 64bit machine to test on, but I do have matlab 2007a.

r.in.mat uses fread() + "long" integers for those things too, so would need to be fixed as well.

thanks for the report, Hamish

in reply to:  1 comment:2 by 1gray, 16 years ago

Replying to hamish:

...

raster/r.out.mat/main.c: The mrows, ncols, format_block, and realflag variables are type "long" and written with fwrite(...,sizeof(long),...);. They should be written as 4 bytes on all platforms. The name_len variable too.

The file should #include <stdint.h>' and use int32_t instead of long' where the bit width is important.

I think it's an easy fix but I have only passing exposure to 64bit programming. I don't have a 64bit machine to test on, but I do have matlab 2007a.

At the very least, the s/long/int32_t/ change (where appropriate) shouldn't break the 32-bit build. Could you check it? (I have access to a 64-bit host, but not to Matlab.)

r.in.mat uses fread() + "long" integers for those things too, so would need to be fixed as well.

...

comment:3 by glynn, 16 years ago

stdint.h and int32_t are C99, and shouldn't be used in GRASS.

The correct solution is to explicitly [de]serialise values rather than reading/writing the in-memory representation directly from/to files.

comment:4 by alexice, 16 years ago

Since I have Matlab (R2007a) and a 64bit machine at hand, please let me know if I can test some versions of the r.out.mat and r.in.mat functions. No problem for me.

alexice

in reply to:  3 comment:5 by 1gray, 16 years ago

Replying to glynn:

stdint.h and int32_t are C99, and shouldn't be used in GRASS.

I stand corrected.

The correct solution is to explicitly [de]serialise values rather than reading/writing the in-memory representation directly from/to files.

I was about to suggest using Gnulib-provided stdint.h (since Gnulib is not a dependency, please check here), but won't the explicit [de]serialisation also solve endianness issues, if any?

BTW, shouldn't the other modules also be checked for similar issues? These [de]serialization routines are, to my mind, good candidates to inclusion into the library.

comment:6 by hamish, 16 years ago

Glynn:

The correct solution is to explicitly [de]serialise values rather than reading/writing the in-memory representation directly from/to files.

If anyone wants to write a patch, feel free.

1grey:

but won't the explicit [de]serialisation also solve endianness issues, if any?

r.in|out.mat's endian code is in a only-just-working state. As the endianness of the file's data is stored in one of the header bits I propose we follow one of Glynn's earlier suggestions and just force r.out.mat to always produce little endian output regardless of platform instead of dynamically picking what to use based on the current platform. With Mac moving to intel this is less of an issue, but since we know it's a problem we might as well take the opportunity to fix that too.

1gray:

BTW, shouldn't the other modules also be checked for similar issues? These [de]serialization routines are, to my mind, good candidates to inclusion into the library.

r.in|out.bin is the main one to worry about. I am not sure but hopefully r.*.gdal and v.*.ogr 64bit+endian stuff is all handled upstream in the (well maintained) support libraries.

Hamish

comment:7 by hamish, 16 years ago

Glynn wrote on grass-dev ml:

Actually, I would suggest using big-endian format. That way, if someone writes code which ignores byte order (i.e. just read directly into memory), you discover it sooner rather than later.

[On a related issue, the UK is at a disadvantage when it comes to writing code which deals with timezones. If you confuse GMT and local time, you won't notice until daylight-saving time starts.]

in reply to:  6 comment:8 by glynn, 16 years ago

Replying to hamish:

The correct solution is to explicitly [de]serialise values rather than reading/writing the in-memory representation directly from/to files.

If anyone wants to write a patch, feel free.

For now, I've simply replaced "long" with "int", as the latter will be 32 bits on nearly all systems.

That should fix the 64-bit problems, but it won't do anything about byte order.

comment:9 by hamish, 16 years ago

Resolution: fixed
Status: newclosed

Thanks Glynn.

Tested* and backported to the 6.3.0 branch.

[*] tested on a 32-bit system, before/after output files are binary equal.

making the two modules big-endian safe is a well documented TODO, so no need to keep this bug report open for that; closing it.

I'd still like to hear feedback from a 64bit user confirming that everything works now.

Hamish

comment:10 by hamish, 16 years ago

ps- for doing the little to big-endian conversion see the commented out code at the end of the r.in.mat/main.c. SwabShort(), SwabLong(), SwabFloat(), SwabDouble()

H

by neteler, 16 years ago

Attachment: roads_64bit.mat.gz added

Spearfish roads raste map, exported on 64bit Linux box (Mandriva 2008.0)

comment:11 by neteler, 16 years ago

I have uploaded the exported Spearfish roads raster map, exported on 64bit Linux box (Mandriva 2008.0).

Markus

in reply to:  11 comment:12 by hamish, 16 years ago

Keywords: r.out.mat endian added

Replying to neteler:

I have uploaded the exported Spearfish roads raster map, exported on 64bit Linux box (Mandriva 2008.0).

I will test the file, but I think this is solved. Specifically the 64 bit error should be solved by:

Glynn:

For now, I've simply replaced "long" with "int", as the latter will be 32 bits on nearly all systems.

That should fix the 64-bit problems, but it won't do anything about byte order.

and thus this bug (wrt 64bit) is closed. A remaining problem is to fix the endian stuff.

or do you experience something else?

Hamish

comment:13 by hamish, 16 years ago

Keywords: 64bit added; endian removed

in reply to:  11 comment:14 by hamish, 16 years ago

Replying to neteler:

I have uploaded the exported Spearfish roads raster map, exported on 64bit Linux box (Mandriva 2008.0).

the file is binary equal to the same made on a 32bit machine and imports fine into matlab 2007a and octave.

Hamish

Note: See TracTickets for help on using tickets.