Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

casacore processed headers when exceptions happen #460

Closed
kswang1029 opened this issue Mar 6, 2020 · 6 comments · Fixed by #843
Closed

casacore processed headers when exceptions happen #460

kswang1029 opened this issue Mar 6, 2020 · 6 comments · Fixed by #843
Assignees
Labels
bug Something isn't working
Milestone

Comments

@kswang1029
Copy link
Contributor

kswang1029 commented Mar 6, 2020

I tested with a set of images with some missing headers (set_spectral_conversion). In general it works fine. A peculiar thing I noticed is that depending on cases, casacore may "smartly" fill up missing headers if those can be computed from other relevant headers (e.g., an image with missing OBSGEO but having TELESCOP: casacore will return headers with OBSGEO recovered). Other thing is with a cube in VRAD but with RESTFRQ missing, the casacore returns in FREQ.

test images

@pford could you help?

@kswang1029 kswang1029 added the bug Something isn't working label Mar 6, 2020
@pford
Copy link
Collaborator

pford commented Mar 9, 2020

The backend currently uses casacore::ImageFITSConverter::ImageHeaderToFITS, which was used in carta 0.9 for a standard FITS header in the file info as mentioned in issue #13.

Currently, the backend makes a casacore::Image for the file info. To do this, casacore uses the image-specific libraries (cfitsio, mirlib, hdf5, casacore) to read the headers, filling in missing information as it is able, as you noted. Then the information contained in the casacore::Image is output in FITS header format.

Are you suggesting reverting to the image-specific libs to read the native headers for file info, as the backend did before the ImageHeaderToFITS implementation?

@kswang1029
Copy link
Contributor Author

No. Unified headers do make things simpler in general. We probably need to revisit this when we work on CARTAvis/carta-frontend#787

But for the second case I mentioned, cannot we keep VRAD as the spectral convention instead of FREQ?

@confluence
Copy link
Collaborator

I've noticed some weirdness in the casacore header output while parsing it in the generic scripting wrapper. The returned entries are different to the original header (which is not necessarily a problem), but are also inexplicably different between a FITS file and the HDF5 file converted from that FITS file (when the converted headers should be the same). Almost every HDF5 file I've checked also has at least one entry with a blank key and a value of 1 or 0 (a handful of files have none, or two). That's definitely malformed. I have briefly debugged this and confirmed that we get this entry from the casacore converter. There is no blank attribute present in the HDF5 file. I'm trying to determine if there is a pattern in the attributes -- the files which do not have this entry have very short headers (e.g. various GALFACTS files).

@pford
Copy link
Collaborator

pford commented Jun 16, 2021

@confluence I pushed branch pam/460_hdf5_headers, which takes the Vector<String> of HDF5 attribute headers and parses them into a casacore::FitsKeywordList so that they can be processed the same way as ImageHeaderToFITS (the previous implementation). This strategy does not add/calculate any additional headers that do not exist in the image file, but it ignores the headers without FITS keywords such as the HDF5 converter info. Is this good enough or do you want all HDF5 headers parsed directly into extended file info HeaderEntry?

@confluence
Copy link
Collaborator

I think that we should expose the converter info headers to the user, so they should be included. But if there are any other headers in the HDF5 file which aren't well-formed FITS headers, they should be treated in the same way as they would be in the original FITS file (ignored if you would ignore them in a FITS file).

@Kechil Kechil added this to the v3.0b-1 milestone Jun 23, 2021
@pford
Copy link
Collaborator

pford commented Jun 24, 2021

HDF5 header pull request merged, but this issue was originally about FITS headers. Reopening issue pending FITS header fix in pull request #827.

@pford pford reopened this Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants