#22 closed defect (fixed)
Grass r.out.mat 64bit Matlab reading problem
Reported by: | alexice | Owned by: | |
---|---|---|---|
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)
Change History (15)
follow-up: 2 comment:1 by , 17 years ago
comment:2 by , 17 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.
...
follow-up: 5 comment:3 by , 17 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 , 17 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
comment:5 by , 17 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.
follow-up: 8 comment:6 by , 17 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 , 17 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.]
comment:8 by , 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 , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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 , 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 , 16 years ago
Attachment: | roads_64bit.mat.gz added |
---|
Spearfish roads raste map, exported on 64bit Linux box (Mandriva 2008.0)
follow-ups: 12 14 comment:11 by , 16 years ago
I have uploaded the exported Spearfish roads raster map, exported on 64bit Linux box (Mandriva 2008.0).
Markus
comment:12 by , 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 , 16 years ago
Keywords: | 64bit added; endian removed |
---|
comment:14 by , 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
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