ratt-ru / tigger

Local Sky Model (LSM) manager and FITS image viewer
8 stars 9 forks source link

Saved FITS images from arithmetic operations have bad headers #128

Closed IanHeywood closed 2 years ago

IanHeywood commented 2 years ago

Saving the result of an image arithmetic operation seems to output only a 2D data array in the FITS header. You can see here on the left the header of an input file, and on the right the header of Tigger's output. Note the NAXIS = 2 entry on the right, as well as missing dimensions for NAXIS3 and NAXIS4.

Screen Shot 2021-10-12 at 10 25 10
IanHeywood commented 2 years ago

This causes stuff like this to happen:

$ fitstool.py -z 6000 test.fits 
Warning: No Cattery package found.
If you have Cattery in a non-standard location, please set the MEQTREES_CATTERY_PATH environment
variable to point to it.
WARNING: FITSFixedWarning: The WCS transformation has more axes (4) than the image it is associated with (2) [astropy.wcs.wcs]
Traceback (most recent call last):
  File "/usr/local/bin/fitstool.py", line 7, in <module>
    exec(compile(f.read(), __file__, 'exec'))
  File "/home/ianh/Software/owlcat/Owlcat/bin/fitstool.py", line 31, in <module>
    main()
  File "/home/ianh/Software/owlcat/Owlcat/FitsTool.py", line 612, in main
    world = wcs.wcs_pix2world(pixcoord, 0)  # get WCS of center pixel
  File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1360, in wcs_pix2world
    'output', *args, **kwargs)
  File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1262, in _array_converter
    return _return_single_array(xy, origin)
  File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1241, in _return_single_array
    "of shape (N, {0})".format(self.naxis))
ValueError: When providing two arguments, the array must be of shape (N, 4)
bennahugo commented 2 years ago

it is because 3 and 4 has been filled, so NAXIS must be 4. Thanks for reporting.

On Tue, Oct 12, 2021 at 11:33 AM IanHeywood @.***> wrote:

This causes stuff like this to happen:

$ fitstool.py -z 6000 test.fits Warning: No Cattery package found. If you have Cattery in a non-standard location, please set the MEQTREES_CATTERY_PATH environment variable to point to it. WARNING: FITSFixedWarning: The WCS transformation has more axes (4) than the image it is associated with (2) [astropy.wcs.wcs] Traceback (most recent call last): File "/usr/local/bin/fitstool.py", line 7, in exec(compile(f.read(), file, 'exec')) File "/home/ianh/Software/owlcat/Owlcat/bin/fitstool.py", line 31, in main() File "/home/ianh/Software/owlcat/Owlcat/FitsTool.py", line 612, in main world = wcs.wcs_pix2world(pixcoord, 0) # get WCS of center pixel File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1360, in wcs_pix2world 'output', *args, **kwargs) File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1262, in _array_converter return _return_single_array(xy, origin) File "/home/ianh/.local/lib/python2.7/site-packages/astropy/wcs/wcs.py", line 1241, in _return_single_array "of shape (N, {0})".format(self.naxis)) ValueError: When providing two arguments, the array must be of shape (N, 4)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ska-sa/tigger/issues/128#issuecomment-940837147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6WF6VJWXBFFR2PJRGDUGP6FBANCNFSM5F2DM5RA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

Benjamin Hugo

PhD. student, Centre for Radio Astronomy Techniques and Technologies Department of Physics and Electronics Rhodes University

Junior software developer Radio Astronomy Research Group South African Radio Astronomy Observatory Black River Business Park Observatory Cape Town

razman786 commented 2 years ago

After some light investigation, I found the following:

After computation the following line removed the 4th AXIS from the data: https://github.com/ratt-ru/tigger/blob/8eef32199d3c18ca80a40acb194c572af8126ff7/TigGUI/Images/Manager.py#L483

The trim procedure has this comment: https://github.com/ratt-ru/tigger/blob/8eef32199d3c18ca80a40acb194c572af8126ff7/TigGUI/Images/Manager.py#L471-L472

At this point, the header contains NAXIS = 4, but the shape of the data is 3, thus the hdu output is corrected by astropy to NAXIS = 3, but still contains information at CTYPE4 et al: https://github.com/ratt-ru/tigger/blob/8eef32199d3c18ca80a40acb194c572af8126ff7/TigGUI/Images/Manager.py#L529

If trimarray is deleted from line 483, the data shape and header then match and is written correctly in the FITS file as NAXIS = 4. I cannot see any adverse effects by doing this, but I am mindful of the comments on lines 471 and 472.

@IanHeywood and @bennahugo - any enlightenment on this would be much appreciated?

razman786 commented 2 years ago

@IanHeywood and @bennahugo - I've updated the fix with an initial commit that creates a new header for the computed image. The old header and new header are diff'd, with missing items added/modified back. Saving the image as a FITS file and running fitstool.py -z no longer results in any errors. The final header will need checking, but as many header items as possible are being preserved (no doubt there could be some improvements, i.e. placing back HISTORY with non-ASCII chars) - comments?

razman786 commented 2 years ago

@IanHeywood and @bennahugo - All fixed.

bennahugo commented 2 years ago

Sorry been quite busy with proceedings this side

Off the top of my head no -- data blocks should be left at the dimensionality that it was read in (ie. untrimmed) and an error printed if the reduced shape cannot be matched to any of the operands header shapes. At least this was the previous behavior.

On Fri, May 20, 2022 at 12:42 AM Raz @.***> wrote:

@IanHeywood https://github.com/IanHeywood and @bennahugo https://github.com/bennahugo - All fixed.

— Reply to this email directly, view it on GitHub https://github.com/ratt-ru/tigger/issues/128#issuecomment-1132272737, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6UHZ5JFYZBQMNUYOPTVK27VVANCNFSM5F2DM5RA . You are receiving this because you were mentioned.Message ID: @.***>

--

Benjamin Hugo

PhD. student, Centre for Radio Astronomy Techniques and Technologies Department of Physics and Electronics Rhodes University

Junior software developer Radio Astronomy Research Group South African Radio Astronomy Observatory Black River Business Park Observatory Cape Town

razman786 commented 2 years ago

No worries at all :)

I've diff'd the code section against v.1.5.1 the computeImage method hasn't changed, it has always run trimarray, at least since then.

The matching of headers still operates as you state and will raise an error if it cannot find a matching header from the operands. The matching is via data dimensions, which seems reasonable, but it does call trimarray on the operands data and then matches it against the trimmed computed image.

May be this has been an issue for some time and/or is only seen with NAXIS = 4 headers?

I believe there are two main paths, one is to remove the trimarray leaving the dimensions intact and the other is to fix the header. However, for the first option I note the comment on lines 471-472 (and resulting code) which must have been put into place for a reason (that may not be valid any longer??). I've also implemented the second method, which is to fix the header to match the computed image, but only if there's a mismatch.

My initial concern which I believe is shared looking at your comment, is that the one dimension is cut off from the data and lost. Considering Tigger's code has been like this for some time, I'm not sure if such a concern is valid, hence the second solution to fix the header when there's a mismatch.

In either case it might still be a good idea to catch mismatched headers and data dimensions as astropy will correct NAXIS when creating the primary HDU?

EDIT: fixed typo