Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Sprint:DRP F18-4
-
Team:Data Release Production
Description
Writing an uncompressed Image with an unsigned integer type to FITS with our code results in a BZERO value one less than what CFITSIO or astropy write (without our code intervening in the cast of CFITSIO), and this confuses CFITSIO, astropy, and our own code about the type of the pixels when reading the image back in (because the convention is to use BZERO to distinguish between signed and unsigned types). I'm not sure if the same problem appears in compressed images or not.
I think this can be fixed by modifying the Bzero traits class in fitsCompression.cc, and if that's right most of the work here will just be test code.
Attachments
Issue Links
Activity
Yeah, I think that's the test to do.
Paul Price
added a comment - Yeah, I think that's the test to do. Wrote the test, and the answer is that adding or removing BLANK and ZBLANK has no effect on whether we round-trip NaNs through quantized compression: we never do, and this appears to be intentional.
This line:
https://github.com/lsst/afw/blob/master/src/fits.cc#L1204
passes forceNonfiniteRemoval=(compression.quantizeLevel != 0) to ImageScale::toFits (https://github.com/lsst/afw/blob/master/include/lsst/afw/fitsCompression.h#L285).
Based on the documentation of that argument, I wonder if it should be the opposite - i.e., we should force removal of non-finite values when quantizeLevel == 0, because it's "useful for lossless compression". Of course, that then sounds like it would make lossless compression not round-trip non-finite values, which would also be a problem; maybe we should always set forceNonfiniteRemoval=false, unless the user explicit asks for it in ImageCompressionOptions?
Any recommendation on what the resolution ought to be here?
It looks like it's deeper than this; even if forceNonfiniteRemoval=false, we do not convert NaN to BLANK values appropriately when scaling from floating-point to integer, even when there is no compression, the scaling is trivial and manual (BZERO=0, BSCALE=1), and all actual pixel values are exactly representable in the target pixel type.
Paul Price, I've added a few more commits and created a new ticket for follow-up work (DM-15644). Mind taking a look at those?
My thinking was that ZBLANK shouldn't exist when decompressed is floating-point, because BLANK shouldn't exist for floating-point. And ZBLANK shouldn't exist (for us) when decompressed is integer, because our integer images never have missing pixels. But that view considers quantization and compression to be a single step, and I'm not sure how valid that is.
But I certainly agree that tests are probably the only way to resolve this. I'm guessing we just need to demonstrate that we can round-trip a floating-point image with a NaN in it through quantization and compression?