astrogo / fitsio

fitsio is a pure-Go package to read and write `FITS` files
BSD 3-Clause "New" or "Revised" License
53 stars 24 forks source link

A problem in file "fitsio/decode.go" #22

Closed grewwc closed 7 years ago

grewwc commented 7 years ago

I find "fitsio.Open()" can't open large files. When I use "fitsio.Open()" to open a 1.2 Gigabytes file, I encounter an error, which is:

fitsio: error loading binary table: fitsio: read too few bytes (1073741824). expected 1122704640

I think the problem is in file "decode.go", line 249, which is n, err := dec.r.Read(block) After I change the line to io.ReadAtLeast(dec.r, block, len(block)) , the problem is solved.

I realize 1073741824=2^30, so I think the problem is about the "Read()" function. I'm a newbie to Golang, so I'm not sure if "Read()" funciton can only read up to 2^30 once. In short, I think there should be some changes in source file "decode.go", line 249.

sbinet commented 7 years ago

Hi, Thanks for the report, the fix and for using astrogo :) I was myself still a rather young Go Padawan when I wrote this. 'ReadAtLeast' is indeed correctly taking care of errors such as the system not being able to fill the buffer in one sitting.

I'll fix this shortly. (When in front of a real keyboard.)

Thanks again for the report.

grewwc commented 7 years ago

Dear Sir/Madam,

Thanks very much for your kind reply. It’s a very helpful package and really appreciate for your great work!

Best

Sent from Mail for Windows 10

From: Sebastien Binet Sent: Sunday, August 27, 2017 4:02 AM To: astrogo/fitsio Cc: grewwc; Author Subject: Re: [astrogo/fitsio] A problem in file "fitsio/decode.go" (#22)

Hi, Thanks for the report, the fix and for using astrogo :) I was myself still a rather young Go Padawan when I wrote this. 'ReadAtLeast' is indeed correctly taking care of errors such as the system not being able to fill the buffer in one sitting. I'll fix this shortly. (When in front of a real keyboard.) Thanks again for the report. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

sbinet commented 7 years ago

this should be fixed now in master (via b0d56d4).

Feel free to reopen if that weren't the case.

sbinet commented 7 years ago

also, you should at least update to 4d708e7 that greatly improves read performances for fitsio.Image:

ok      github.com/astrogo/fitsio   97.756s
name                   old time/op    new time/op    delta
ImageReadF64_10-4        4.95µs ± 6%    0.91µs ± 1%  -81.69%  (p=0.000 n=10+7)
ImageReadF64_100-4       44.2µs ± 4%     6.0µs ± 8%  -86.41%  (p=0.000 n=8+10)
ImageReadF64_1000-4       431µs ± 7%      58µs ± 5%  -86.58%  (p=0.000 n=10+10)
ImageReadF64_10000-4     4.38ms ± 4%    0.58ms ± 7%  -86.86%  (p=0.000 n=10+10)
ImageReadF64_100000-4    46.5ms ± 5%     6.1ms ± 3%  -86.84%  (p=0.000 n=10+10)

name                   old alloc/op   new alloc/op   delta
ImageReadF64_10-4          480B ± 0%      160B ± 0%  -66.67%  (p=0.000 n=10+10)
ImageReadF64_100-4       3.36kB ± 0%    0.88kB ± 0%  -73.81%  (p=0.000 n=10+10)
ImageReadF64_1000-4      32.2kB ± 0%     8.1kB ± 0%  -74.88%  (p=0.000 n=10+10)
ImageReadF64_10000-4      320kB ± 0%      80kB ± 0%  -74.99%  (p=0.000 n=10+10)
ImageReadF64_100000-4    3.20MB ± 0%    0.80MB ± 0%  -75.00%  (p=0.000 n=7+10)

name                   old allocs/op  new allocs/op  delta
ImageReadF64_10-4          43.0 ± 0%      12.0 ± 0%  -72.09%  (p=0.000 n=10+10)
ImageReadF64_100-4          403 ± 0%       102 ± 0%  -74.69%  (p=0.000 n=10+10)
ImageReadF64_1000-4       4.00k ± 0%     1.00k ± 0%  -74.97%  (p=0.000 n=10+10)
ImageReadF64_10000-4      40.0k ± 0%     10.0k ± 0%  -75.00%  (p=0.000 n=10+10)
ImageReadF64_100000-4      400k ± 0%      100k ± 0%  -75.00%  (p=0.000 n=10+10)