Always map NaN to ZBLANK in quantization when nullcheck is enabled. #107
Conversation
This remaps NaN to ZBLANK whenever nullvalue is _something_ (even if it's not NaN) and hence checking for nulls is enabled. Note that prior to this change, passing nullvalue=NaN had no effect because NaN != NaN.
beckermr
left a comment
beckermr
left a comment
There was a problem hiding this comment.
Nice patch @TallJimbo! Are we worried that at higher optimization levels the compiler might optimize away the comparisons in the if statements?
I wonder if maybe a more explicit check for nan using the fnan function would be better?
|
I'm not super familiar with the effects of higher optimization levels, but any level that could optimize that away sounds like one that is basically asserting that NaNs cannot exist. |
|
Me neither. Hopefully they don't do that. |
|
Hi @TallJimbo. Can you explain how you're actually getting NaNs to appear in the pixel arrays (fdata, rowpix, etc) inside quantize.c? In my tests, by the time imcompress.c functions call quantize.c, the NaNs are replaced with either of the FLOATNULLVALUE or DOUBLENULLVALUES placeholders defined in fitsio.h. Therefore I'm not seeing where quantize.c is actually trying to perform an "NaN == NaN" comparison. |
|
It's possible I misdiagnosed this; I actually ended up working around this problem (by replacing my NaNs with -infs) and then went back and made this fix later, without testing that this fix removed the need for that workaround. But one unusual thing I was doing in the tests that demonstrated the problem was passing a negative value to |
|
If no NaNs are detected, then it may not need to add a ZBLANK keyword. FWIW, in my test with NaNs and a negative quantization level (-4.0) using our 'fpack' utility, it still properly produces ZBLANK and successfully compresses the image. But if you want to send me a specific test case where the compression is failing, I'd be happy to try it out. |
|
I'll try to reconstruct (and simplify) my failing test; might take me a little while. |
3 participants
This remaps NaN to ZBLANK whenever nullvalue is something (even if it's not NaN) and hence checking for nulls is enabled. Note that prior to this change, passing nullvalue=NaN had no effect because NaN != NaN, and hence any image that naturally had NaNs had to be modified in order to be saved with quantized compression, since otherwise the NaNs destroy everything.