Check beam headers for casacore::FITSImage #938
Conversation
src/ImageData/FitsLoader.h
Outdated
| // Get headers if they exist | ||
| char value[80]; | ||
| int bmaj_status(0), bmin_status(0), bpa_status(0); | ||
| fits_read_keyword(fptr, "BMAJ", value, nullptr, &bmaj_status); |
There was a problem hiding this comment.
This is reading through the file three times, isn't it? Could we not rather loop through keywords and ensure that bmaj, bmin and BPA exist? And then break if they do. That would read through the file at most once. This is only a concern when you have files with very long headers, but unfortunately those are not uncommon
There was a problem hiding this comment.
@pford what about this instead?
// Get headers if they exist
char key_name[FLEN_KEYWORD];
char value[FLEN_VALUE];
bool bmaj_found = false, bmin_found = false, bpa_found = false;
int key_num = 1;
while (status == 0 && !(bmaj_found && bmin_found && bpa_found)) {
fits_read_keyn(fptr, key_num, key_name, value, nullptr, &status);
bmaj_found |= strncmp(key_name, "BMAJ", FLEN_KEYWORD) == 0;
bmin_found |= strncmp(key_name, "BMIN", FLEN_KEYWORD) == 0;
bpa_found |= strncmp(key_name, "BPA", FLEN_KEYWORD) == 0;
key_num++;
}
// Close file
status = 0;
fits_close_file(fptr, &status);
bool beam_headers_missing = !(bmaj_found && bmin_found && bpa_found);Alternatively, you can use something like fits_find_nextkey and an argument of ["BMAJ", "BMIN", "BPA"] for inclist to search for the headers in one iteration.
There was a problem hiding this comment.
I thought fits_read_record would give better performance, no need to parse keyword/value/comment since the keyword is at the beginning.
|
Tests are failing on MacOS because accessing the M17_SWex.fits image exceeds the time limit of by up to 10 ms, e.g. |
I don't think that's what's happening. I added a timer to the start and end of That looks like the tests are giving up after just over 2100 ms because it hasn't received a response at all. |
|
I tried the example image from the user who reported this issue. The backend crashed when requesting file info/header. Please refer to the attachment for the image. backend console log:
Crash report from macOS: Process: carta_backend [24372] Date/Time: 2021-10-29 11:25:21.323 +0800 Sleep/Wake UUID: C175BB0A-4689-478B-8B1C-94BECD82F7E4 Time Awake Since Boot: 70000 seconds System Integrity Protection: enabled Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_CRASH (SIGABRT) Application Specific Information: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread Thread 1: Thread 2: Thread 3: Thread 0 crashed with X86 Thread State (64-bit): Logical CPU: 0 Binary Images: External Modification Summary: VM Region Summary: REGION TYPE SIZE COUNT (non-coalesced) |
|
@kswang1029 this branch crashes on MacOS. I am working on a fix then will test your image (works on Ubuntu). |
|
@kswang1029 fixed MacOS error, all tests now pass. Please retest this PR. |
|
@pford thanks. The crash issue is fixed. 👍 The CROTA index is missing. Could you have another look? Left: CARTA, Right: ds9 |
|
@pford can't we just use edit: don't worry, have made the change and merged this branch 😄 |
Closes #935 .
casacore::FITSImage uses the HISTORY headers for BMAJ, BMIN, and BPA when not set in regular headers. This is because the AIPS package stores beam info in HISTORY.
When FitsLoader loads a casacore::FITSImage with a single beam, it will use cfitsio to retrieve the non-history headers and, if not found, remove the restoring beam in the FITSImage (stored in its ImageInfo).
There is a bit of a performance hit (when there are numerous headers) while these headers are being checked for a FITSImage, used to generate the file info and to load the image. Any better ideas?