Check whether the FITS beam table consists of 64-bit floats

[markccchiang]

Description

• What is implemented or fixed? Mention the linked issue(s), if any. Closes #1166.

• How does this PR solve the issue? Give a brief summary. Using cfitsio to check whether the FITS beam table consists of 64-bit floats. If so, open the file via CartaFitsImage object. Otherwise, open the file with casacore::FITSImage.

• Are there any companion PRs (frontend, protobuf)? No.

• Is there anything else that testers should know (e.g. exactly how to reproduce the issue)? The crash could be avoided when opening the FITS file with beams table consisting of 64-bit floats.

Checklist

• [x] changelog updated ~/ no changelog update needed~
• [ ] e2e test passing ~/ added corresponding fix~
• [x] ~protobuf updated to the latest dev commit /~ no protobuf update needed
• [ ] added reviewers and assignee
• [ ] added ZenHub estimate, milestone, and release

[pford]

CartaFitsImage::ReadBeamsTable reads these columns as TFLOAT. Shouldn't this be fixed as well, to determine the column type then read the values as TFLOAT or TDOUBLE? casacore::Quantity expects a double value with a string unit so that is no problem.

[markccchiang]

CartaFitsImage::ReadBeamsTable reads these columns as TFLOAT. Shouldn't this be fixed as well, to determine the column type then read the values as TFLOAT or TDOUBLE? casacore::Quantity expects a double value with a string unit so that is no problem.

It seems in function fits_read_col if the reading data type is inconsistent with the writing data type of an array we declared, an error or crash would happen. So maybe it is ok to keep the original way that we convert all beam parameters to the float type(?)

[pford]

CartaFitsImage::ReadBeamsTable reads these columns as TFLOAT. Shouldn't this be fixed as well, to determine the column type then read the values as TFLOAT or TDOUBLE? casacore::Quantity expects a double value with a string unit so that is no problem.

It seems in function fits_read_col if the reading data type is inconsistent with the writing data type of an array we declared, an error or crash would happen. So maybe it is ok to keep the original way that we convert all beam parameters to the float type(?)

I am suggesting that we determine the column datatype (fits_get_coltype) then read the data as that type, rather than losing precision converting double to float and back to double.

// for bmaj
std::string bmaj_name("BMAJ");
fits_get_colnum(fptr, casesen, bmaj_name.c_str(), &colnum, &status);
fits_get_coltype(fptr, colnum, &datatype, &repeat, &width, &status);

std::vector<float> bmaj_f;
std::vector<double> bmaj_d;

if (datatype == TFLOAT) {
    bmaj_f.resize(nrow);
    fits_read_col(fptr, datatype, colnum, firstrow, firstelem, nrow, fnulval, bmaj_f.data(), &anynul, &status);
} else if (datatype == TDOUBLE) {
    bmaj_d.resize(nrow);
    fits_read_col(fptr, datatype, colnum, firstrow, firstelem, nrow, fnulval, bmaj_d.data(), &anynul, &status);
}

Better to put this in a method, something like GetColumnData(fitsfile* fptr, const std::string& name, int nrow, int& datatype, std::vector<float>& values_f, std::vector<double>& values_d) where the returned datatype tells you which returned vector to use to set the Quantities (or do not return datatype and just check which vector is not empty). Then call the method for "BMAJ", "BMIN", "BPA".

[markccchiang]

Better to put this in a method, something like GetColumnData(fitsfile* fptr, const std::string& name, int nrow, int& datatype, std::vector<float>& values_f, std::vector<double>& values_d) where the returned datatype tells you which returned vector to use to set the Quantities (or do not return datatype and just check which vector is not empty). Then call the method for "BMAJ", "BMIN", "BPA".

Thank you for the suggestion! I refactored this part of the code in a slightly different way, added a check for the data type, and got a data array accordingly. Hope it is easier to read.

[github-actions[bot]]

Package	Line Rate	Health
src.Cache	66%	➖
src.DataStream	52%	➖
src.FileList	67%	➖
src.Frame	50%	➖
src.HttpServer	43%	➖
src.ImageData	28%	❌
src.ImageFitter	89%	✔
src.ImageGenerators	44%	➖
src.ImageStats	76%	✔
src.Logger	44%	➖
src.Main	54%	➖
src.Region	18%	❌
src.Session	29%	❌
src.Table	52%	➖
src.ThreadingManager	87%	✔
src.Timer	85%	✔
src.Util	49%	➖
Summary	38% (6965 / 18364)	❌