Unable to Load FITS files with NaN values about js9 HOT 12 CLOSED

@Mulan-94 @o-smirnov

OK, there are two problems here: at first I thought that wcsinit() had to be passed a larger value for hlength (max length to check for a header param in case END is not found, default 256000). But now I believe that the current hlength is fine in your case: all of the relevant header parameters are way before the 256000 byte limit. That is, even with this too-small limit, all params should be found correctly.

More problematic is the Emsripten stack limit problem:

emscripten-core/emscripten#11135

By default, Emscripten has a 5Mb stack limit. Strings are passed on the stack using 4 * string size + 1 bytes, so your 2.8Mb header requires > 10Mb of stack space and ... SEGV time.

I don't want to allocate a semi-infinite amount of stack space for the occasional huge size header FITS file, so I'm gonna have to figure out how to call wcsinit() more cleverly. Stay tuned ...

from js9.

Still a way to go, but the heap-passing technique is starting to look promising ...

from js9.

@Mulan-94 Hi Lexy, I'll need some more info, because below is a test image containing NaN values that loads correctly. If you have an image containing NaN values that crashes, could I please get a copy? Or, if you are using JS9.MaskImage(), can I get a copy of both the mask and the image being masked?

from js9.

@Mulan-94 Also, once we have a file that reproduces the problem, we'll need to know how the NaN values are made. I see that the Posix nan() call takes an argument and I just passed a NULL. Of course, there are other ways to make a NaN. Without any data whatsoever, I see two possibilities:

1. if the segv is happening in the CFITIO library, we can run the ftools fverify on the file, test against a CFITSIO desktop program, perhaps submit a bug report to HEASARC

2. if the segv is happening in Javascript, we'll want to make sure that the standard Number.isNaN() is handling your NaN values correctly, we might require a work-around.

from js9.

@ericmandel sorry just seeing this, I'm emailing you the image now

from js9.

@Mulan-94

When I open CYG-FPOL-FLO.FITS using JS9 (Desktop version), I get the following error:

And looking at the file, I see the header is really long: 2822400 bytes or 35280 80-byte header entries, almost all of it HISTORY keywords. Following wcslib defaults (and after long conversation with Jessica Mink, wcslib author), I set a max header size to 256000 bytes, or 3200 header entries.

I'm not sure what to do about this. Off-hand I forget what the trade-offs are for increasing the wcsinit max size and I will look into this. In the meantime, can you tell me if this is an anomaly or do you expect files header of this size typically?

cc: @o-smirnov

from js9.

I'm not sure actually, let's wait for @o-smirnov on this one

from js9.

@Mulan-94 @o-smirnov I've sent email to Jessica Mink (instead of walking over to her office, sigh), asking for advice on increasing the max header parameter. The relevant code looks benign, so I hope we will be able to do this. I'd probably try 5Mb or 10Mb to start ...

from js9.

Thanks for looking into this @ericmandel! That header is pretty excessive but, sadly, not exceptional for radio images produced by AIPS. It likes to be very verbose about its data processing history. So while we can easily work around it in each individual case, this is something that will continue to trip the unwary user...

from js9.

@o-smirnov @Mulan-94 That's fine, I have practically no experience with radio data and was hoping it was a mistake!

I just have to copy the header string to the Emscripten heap (and pass a pointer) instead of passing the string directly on the stack ... once I get that technique working, it should work forever ...

from js9.

@Mulan-94 @o-smirnov OK, we're all set and updated in GitHub. Thanks, Lexy, for pointing this one out, it was a very interesting bug!

from js9.

Great! and thanks @ericmandel for taking your time to look into it so quickly.

from js9.