Incorrect checksum/datasum in some cases for compressed FITS images?

While experimenting with checksums for compressed FITS images, I came across the following strange behavior. First, I load up the comp.fits file from our test suite and write it out, forcing a checksum calculation:

In [1]: import numpy as np
   ...: from astropy.io import fits

In [2]: from astropy.io.fits.hdu.base import _ValidHDU
   ...: _ValidHDU._get_timestamp = lambda self: "2013-12-20T13:36:10"  # Ensure deterministic checksums

In [3]: from astropy.utils.data import get_pkg_data_filename
   ...: 

In [4]: hdulist = fits.open(get_pkg_data_filename('data/comp.fits', package='astropy.io.fits.tests'))
   ...: hdulist.writeto('test_checksum.fits', overwrite=True, checksum=True)

Then I can try and load it up as a raw binary table:

In [5]: bintable_hdu = fits.open('test_checksum.fits', disable_image_compression=True)[1]

In [6]: bintable_hdu.header['CHECKSUM']
Out[6]: 'ZeAbdb8aZbAabb7a'

In [7]: bintable_hdu.header['DATASUM']
Out[7]: '113055149'

and I can also load it up decompressed:

In [8]: comp_image_hdu = fits.open('test_checksum.fits')[1]

In [9]: comp_image_hdu.header['CHECKSUM']
Out[9]: 'ZeAbdb8aZbAabb7a'

In [10]: comp_image_hdu.header['DATASUM']
Out[10]: '113055149'

The checksums and datasums are the same, but they shouldn't be - the ones for bintable_hdu should be the ones for the actual binary table in the FITS file, while the comp_image_hdu ones should be the ones for the decompressed image.

Furthermore, the image checksums and datasums should be present in the binary table as ZHECKSUM and ZDATASUM but they are not.

It looks like the checksum and datasum for comp_image_hdu should actually be the following:

In [14]: image_hdu = fits.ImageHDU(comp_image_hdu.data, header=comp_image_hdu.header)
    ...: image_hdu.writeto('image.fits', overwrite=True, checksum=True)

In [15]: image_hdu.header['CHECKSUM']
Out[15]: 'D2GXD29VD2EVD29V'

In [16]: image_hdu.header['DATASUM']
Out[16]: '2189405276'

I am currently working on a possible refactor of CompImageHDU and in my version I get the D2GXD29VD2EVD29V checksum, so I wanted to double check whether we agree that the current behavior is buggy and that the following test is incorrect:

    def test_compressed_image_data(self):
        with fits.open(self.data("comp.fits")) as h1:
            h1.writeto(self.temp("tmp.fits"), overwrite=True, checksum=True)
            with fits.open(self.temp("tmp.fits"), checksum=True) as h2:
                ...
                assert "CHECKSUM" in h2[1].header
                assert h2[1].header["CHECKSUM"] == "ZeAbdb8aZbAabb7a"
                assert "DATASUM" in h2[1].header
                assert h2[1].header["DATASUM"] == "113055149"

Specifically, the last checksum and datasum should be updated to D2GXD29VD2EVD29V and 2189405276?

It seems that it is also a bug that ZHECKSUM and ZDATASUM are missing from the binary table on disk - this is also fixed in my branch but I wanted to check that we agree this is also a bug in the current code?

Any ideas on how to validate the checksums with third-party tools? (fitsverify doesn't throw an error because the actual binary table checksum is correct and I don't think it checks ZHECKSUM, which is missing anyway)

I agree with you, CHECKSUM/DATASUM from the image extension should have been renamed to ZCHECKSUM/ZDATASUM when creating the compressed HDU. However I'm not sure about having both the checksum for the compressed image (ZCHECKSUM) and for the bintable (CHECKSUM). The latter seems useless. It seems fitsio supports both tile compression and checksums, so I will try to create a file and see what they put in it ;).

Hmm so fitsio just writes CHECKSUM/DATASUM:

ZIMAGE  =                    T / extension contains compressed image
ZSIMPLE =                    T / file does conform to FITS standard
ZBITPIX =                  -32 / data type of original image
ZNAXIS  =                    2 / dimension of original image
ZNAXIS1 =                 1000 / length of original image axis
ZNAXIS2 =                 1000 / length of original image axis
ZTILE1  =                  100 / size of tiles to be compressed
ZTILE2  =                  100 / size of tiles to be compressed
ZQUANTIZ= 'SUBTRACTIVE_DITHER_1' / Pixel Quantization Algorithm
ZDITHER0=                 6234 / dithering offset when quantizing floats
ZCMPTYPE= 'RICE_1  '           / compression algorithm
ZNAME1  = 'BLOCKSIZE'          / compression block size
ZVAL1   =                   32 / pixels per block
ZNAME2  = 'BYTEPIX '           / bytes per pixel (1, 2, 4, or 8)
ZVAL2   =                    4 / bytes per pixel (1, 2, 4, or 8)
CHECKSUM= '93N2A0N030N090N0'   / HDU checksum updated 2023-05-24T16:36:36
DATASUM = '4121375213'         / data unit checksum updated 2023-05-24T16:36:36

Quick update on this: the top example seem to have changed behavior at some point, now all checksums are identical and all datasum too. For convenience, I'll reproduce the example as a standalone script

from astropy.io import fits
from astropy.utils.data import get_pkg_data_filename
from astropy.io.fits.hdu.base import _ValidHDU
_ValidHDU._get_timestamp = lambda self: "2013-12-20T13:36:10"  # Ensure deterministic checksums


hdulist = fits.open(get_pkg_data_filename('data/comp.fits', package='astropy.io.fits.tests'))
hdulist.writeto('test_checksum.fits', overwrite=True, checksum=True)
bintable_hdu = fits.open('test_checksum.fits', disable_image_compression=True)[1]
checksum1 = bintable_hdu.header['CHECKSUM']
datasum1 = bintable_hdu.header['DATASUM']

comp_image_hdu = fits.open('test_checksum.fits')[1]
checksum2 = comp_image_hdu.header['CHECKSUM']
datasum2 = comp_image_hdu.header['DATASUM']

image_hdu = fits.ImageHDU(comp_image_hdu.data, header=comp_image_hdu.header)
checksum3 = image_hdu.header['CHECKSUM']
datasum3 = image_hdu.header['DATASUM']

print(f"{checksum1=}, {datasum1=}")
print(f"{checksum2=}, {datasum2=}")
print(f"{checksum3=}, {datasum3=}")

current output (as of astropy-6.1.dev489+ga7cd7bea56)

checksum1='ZeAbdb8aZbAabb7a', datasum1='113055149'
checksum2='ZeAbdb8aZbAabb7a', datasum2='113055149'
checksum3='ZeAbdb8aZbAabb7a', datasum3='113055149'

@astrofrog

I don't think so. If I understand correctly, the problem seem to have gotten worse, actually. But @astrofrog 's PR might fix it ? #15474

@saimn

@saimn - we should discuss the checksum question here! :)

@neutrinoceros - I don't think the example you showed is worse as such than what I showed, in the last case the checksum is the same because you put it in the header, and it did not get recomputed?

Oh, right, I didn't realize this part was important.

image_hdu.writeto('image.fits', overwrite=True, checksum=True)

Nevermind !

Answering to #15474 (comment):

some tests on main: https://github.com/saimn/misc-astro/blob/main/astropy/Datasum.ipynb
and I did not force the date so we should consider the value of DATASUM (CHECKSUM includes DATASUM and the header).

So currently on main DATASUM is computed on the table data (i.e. compressed data), I think this is correct. And this is also what fitsio does. Having DATASUM/CHECKSUM computed on the table HDU allows to verify the checksum without having to decompress the data.

The problem comes from ZHECKSUM/ZDATASUM, which I think is ill-defined. The Standard just says:

The value field of this keyword shall contain a character string that gives the value of the CHECKSUM/DATASUM keyword (see Sect. 4.4.2.7) in the original uncompressed FITS HDU.

So if an image HDU containing CHECKSUM/DATASUM is compressed, the keywords should be moved to ZHECKSUM/ZDATASUM. But in general we deal only with a CompImageHDU, without creating an ImageHDU before compression or after decompression. If we were to decompress to an ImageHDU then its header could contain the ZHECKSUM/ZDATASUM renamed to CHECKSUM/DATASUM. But I don't think it makes sense to do that in the header of the CompImageHDU.

This can be seen on my notebook: if the data is loaded before computed the checksum, then the file will have both ZHECKSUM/ZDATASUM and CHECKSUM/DATASUM. It seems weird that loading the data results in a different file. And in that case, when reading the file the ZHECKSUM/ZDATASUM values replace the CHECKSUM/DATASUM ones, which is confusing.

So I think we should

compute DATASUM/CHECKSUM table data
keep DATASUM/CHECKSUM from the table data when reading
and probably forget that ZHECKSUM/ZDATASUM exist 😅 (I don't see what we can do with those cards, they are useless. If they are present in a compressed HDU, fine, we can just ignore them)

I can see why pragmatically it probably makes sense to do as you are suggesting. Idealistically, as CompImageHDU is a subclass of ImageHDU and is meant to represent the decompressed image HDU, the CHECKSUM and DATASUM in the header should be the ones of the decompressed image. From my personal point of view I don't think ZHECKSUM and ZDATASUM are useless - a user could always check that the checksum and datasum are correct in the decompressed data. With your approach we will need to explicitly disable CompImageHDU._update_checksum as it would otherwise compute the image one as in ImageHDU.

In any case, what I will do for now is make #15474 behave as it does on main, and if/once that PR is merged, I will think some more about whether I still think we should change the behavior.

Looking at the code on main, I think the intent of the code was to properly deal with ZHECKSUM and ZDATASUM. For instance, on main, we transfer ZHECKSUM and ZDATASUM to CHECKSUM and DATASUM on read:

astropy/astropy/io/fits/hdu/compressed/header.py

Lines 451 to 457 in e581eab

    
           # Move the ZHECKSUM and ZDATASUM cards to the image header 
        
           # as CHECKSUM and DATASUM 
        
           if "ZHECKSUM" in bintable_header: 
        
               image_header.set("CHECKSUM", bintable_header["ZHECKSUM"], hcomments["ZHECKSUM"]) 
        
           if "ZDATASUM" in bintable_header: 
        
               image_header.set("DATASUM", bintable_header["ZDATASUM"], hcomments["ZDATASUM"])

and we transfer them the other way on write:

astropy/astropy/io/fits/hdu/compressed/header.py

Lines 956 to 971 in e581eab

    
           # Move CHECKSUM and DATASUM cards from the image header to the 
        
           # table header as XHECKSUM and XDATASUM cards. 
        
           if "CHECKSUM" in image_header: 
        
               bintable_header.set( 
        
                   "ZHECKSUM", 
        
                   image_header["CHECKSUM"], 
        
                   image_header.comments["CHECKSUM"], 
        
               ) 
        
           if "DATASUM" in image_header: 
        
               bintable_header.set( 
        
                   "ZDATASUM", 
        
                   image_header["DATASUM"], 
        
                   image_header.comments["DATASUM"], 
        
               )

We also compute the decompressed checksum and datasum when writing out a file:

astropy/astropy/io/fits/hdu/compressed/compressed.py

Lines 866 to 883 in e581eab

    
           # Shove the image header and data into a new ImageHDU and use that 
        
           # to compute the image checksum 
        
           image_hdu = ImageHDU(data=self.data, header=self.header) 
        
           image_hdu._update_checksum(checksum) 
        
           if "CHECKSUM" in image_hdu.header: 
        
               # This will also pass through to the ZHECKSUM keyword and 
        
               # ZDATASUM keyword 
        
               self._image_header.set( 
        
                   "CHECKSUM", 
        
                   image_hdu.header["CHECKSUM"], 
        
                   image_hdu.header.comments["CHECKSUM"], 
        
               ) 
        
           if "DATASUM" in image_hdu.header: 
        
               self._image_header.set( 
        
                   "DATASUM", 
        
                   image_hdu.header["DATASUM"], 
        
                   image_hdu.header.comments["DATASUM"], 
        
               )

and the comment even indicates that it should end up as ZHECKSUM and ZDATASUM in the file. I think there is a bug that if you don't update the data or header then the checksum might not actually be computed. This is because in this case self._has_data is False so the image HDU checksum never gets computed.

In fact, it your notebook, it is actually doing the right thing on reading in the file:

In this case the checksum in the decompressed header is actually the one from ZHECKSUM and ZDATASUM!

The bottom line is that I'm certain now at this point that on main we are trying to do the right thing which is:

CompImageHDU.header['CHECKSUM'] should be the one for the decompressed data (which is the case in the comp-ref2.fits example in your notebook. This then round-trips to disk via ZHECKSUM.
BinTableHDU.header['CHECKSUM'] should be the compressed checksum and should not appear in CompImageHDU.header

However there is a bug in astropy main which is that weird things happen if you read a file and write it straight out without setting the data. The behavior is correct if you do change the data.

CompImageHDU is meant to be an ImageHDU (it wasn't fully on main but it does become so in my refactoring PR) so the checksum in CompImageHDU should be as if it was an ImageHDU.

To demonstrate the bug in main:

326be1c

if you run the tests with these changes you will get:

    def test_compressed_image_data(self):
        with fits.open(self.data("comp.fits")) as h1:
            h1[1].data = h1[1].data.copy()
            h1.writeto(self.temp("tmp.fits"), overwrite=True, checksum=True)
            with fits.open(self.temp("tmp.fits"), checksum=True) as h2:
                assert np.all(h1[1].data == h2[1].data)
                assert "CHECKSUM" in h2[0].header
                assert h2[0].header["CHECKSUM"] == "D8iBD6ZAD6fAD6ZA"
                assert "DATASUM" in h2[0].header
                assert h2[0].header["DATASUM"] == "0"
                assert "CHECKSUM" in h2[1].header
>               assert h2[1].header["CHECKSUM"] == "ZeAbdb8aZbAabb7a"
E               AssertionError: assert 'D2GXD29VD2EVD29V' == 'ZeAbdb8aZbAabb7a'
E                 
E                 - ZeAbdb8aZbAabb7a
E                 + D2GXD29VD2EVD29V

Code/astropy/astropy/io/fits/hdu/compressed/tests/test_checksum.py:23: AssertionError

That is, the checksum in CompImageHDU.header is now the image HDU checksum! (see my examples at the top of this issue where I showed D2GXD29VD2EVD29V was the image checksum)

See #16957

Yes, depending on whether you load the data or not you will get a different checksum in CompImageHDU.header, that's clearly a bug and apart from that I agree that the code on main is trying to handle ZHECKSUM and ZDATASUM.

The bottom line is that I'm certain now at this point that on main we are trying to do the right thing which is:

CompImageHDU.header['CHECKSUM'] should be the one for the decompressed data (which is the case in the comp-ref2.fits example in your notebook. This then round-trips to disk via ZHECKSUM.

BinTableHDU.header['CHECKSUM'] should be the compressed checksum and should not appear in CompImageHDU.header

Well now that we understand the current behavior, what should CompImageHDU.header['CHECKSUM'] contain is the main question.
We could do as you suggest, or we could keep the checksum of the compressed data. I'm leaning towards the second option for several reasons:

we can add checksum to a file without computing the checksum of the image data (ZHECKSUM/ZDATASUM). Those keywords are optional and their interest is not so clear. With the 1st option what should we do? I don't think we want to enforce computing the image data checksums (as it adds some overhead), so should we add a new option to compute those if someone want to ? If we do not compute them by default then CompImageHDU.header['CHECKSUM'] would be empty and the table CHECKSUM which is visible in the file would be hidden ?
should we verify both compressed and uncompressed checksum if both are present ?
I don't see the point of having 2 layers of checksum, so handling both just adds unnecessary complexity and possible confusion for people that won't understand why CompImageHDU.header['CHECKSUM'] is not the CHECKSUM they see in their file.
checksum are computed when writing a file and verified when reading. It's really a feature related to the file on disk so what matters to me here is the compressed data.
and now that I think about it, I think the tile compression can be lossy, e.g. when using quantization. So we cannot assume that the image data checksum will be correct after decompression ?

I see your point about computing the image checksum being expensive if the data is not already decompressed. So just to be clear, would you be happy with the following? (not mentioning DATASUM but it would behave the same as CHECKSUM)

By default, if writing a file with checksum=True, the binary table checksum is written to the file on disk as CHECKSUM in the binary table header. No ZCHECKSUM is written out. When reading in that file, the table checksum is optionally verified (depending on whether requested by the user), and the decompressed header in CompImageHDU.header would not contain any checksum (I think it's important not to have the CHECKSUM in the decompressed header be the table checksum - better to not have it at all).
If a file contains ZHECKSUM, it is moved to CompImageHDU.header as CHECKSUM, but that checksum is not validated by default.
In the future we could choose to allow the user to compute the decompressed checksum and write it to CHECKSUM in the decompressed header and get stored as ZHECKSUM on disk in the binary table (we could always refuse to for lossy algorithms). We could also add a way to verify the decompressed checksum but as you said it wouldn't work for lossy algorithms.

Does this sound ok? If so, I'll update my refactoring PR to follow this behavior.

Ok sounds good, but if we do 2) I think we have to do 3), i.e. if CHECKSUM is present in CompImageHDU.header it must be written as ZHECKSUM. It's already done on main, so we should keep that. And users already have a way to have the image CHECKSUM, which is to create a CompImageHDU from an ImageHDU containing the checksum, which is consistent with the Standard definition of ZHECKSUM.

Navigation Menu

Repository navigation

Incorrect checksum/datasum in some cases for compressed FITS images? #14753

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	# Move the ZHECKSUM and ZDATASUM cards to the image header
	# as CHECKSUM and DATASUM
	if "ZHECKSUM" in bintable_header:
	image_header.set("CHECKSUM", bintable_header["ZHECKSUM"], hcomments["ZHECKSUM"])

	if "ZDATASUM" in bintable_header:
	image_header.set("DATASUM", bintable_header["ZDATASUM"], hcomments["ZDATASUM"])

	# Move CHECKSUM and DATASUM cards from the image header to the
	# table header as XHECKSUM and XDATASUM cards.

	if "CHECKSUM" in image_header:
	bintable_header.set(
	"ZHECKSUM",
	image_header["CHECKSUM"],
	image_header.comments["CHECKSUM"],
	)

	if "DATASUM" in image_header:
	bintable_header.set(
	"ZDATASUM",
	image_header["DATASUM"],
	image_header.comments["DATASUM"],
	)

	# Shove the image header and data into a new ImageHDU and use that
	# to compute the image checksum
	image_hdu = ImageHDU(data=self.data, header=self.header)
	image_hdu._update_checksum(checksum)
	if "CHECKSUM" in image_hdu.header:
	# This will also pass through to the ZHECKSUM keyword and
	# ZDATASUM keyword
	self._image_header.set(
	"CHECKSUM",
	image_hdu.header["CHECKSUM"],
	image_hdu.header.comments["CHECKSUM"],
	)
	if "DATASUM" in image_hdu.header:
	self._image_header.set(
	"DATASUM",
	image_hdu.header["DATASUM"],
	image_hdu.header.comments["DATASUM"],
	)

Incorrect checksum/datasum in some cases for compressed FITS images? #14753

Description

Activity

saimn commented on May 23, 2023

saimn commented on May 24, 2023

neutrinoceros commented on Feb 21, 2024

pllim commented on Feb 21, 2024

neutrinoceros commented on Feb 21, 2024

astrofrog commented on Sep 3, 2024

neutrinoceros commented on Sep 4, 2024

saimn commented on Sep 5, 2024

astrofrog commented on Sep 6, 2024

astrofrog commented on Sep 6, 2024

astrofrog commented on Sep 6, 2024

astrofrog commented on Sep 6, 2024

saimn commented on Sep 6, 2024

astrofrog commented on Sep 9, 2024

saimn commented on Sep 10, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions