nlesc-dirac / sagecal

SAGECal is a fast, memory efficient and GPU accelerated radio interferometric calibration program. It supports all source models including points, Gaussians and Shapelets. Distributed calibration using MPI and consensus optimization is enabled. Both spectral and spatial priors can be used as constraints. Tools to build/restore sky models are included.
http://sagecal.sourceforge.net
GNU General Public License v2.0
12 stars 8 forks source link

Potential Redundant Conversion of RA/DEC to Radians in uvwriter.cpp #209

Closed SoniaGhosh24 closed 2 months ago

SoniaGhosh24 commented 2 months ago

Hello,

I am using uvwriter.cpp script. While trying to understand the script, I came across the following part where the phase center coordinates (Right Ascension (RA) and Declination (DEC)) are read from the MS:

MSField _field = Table(ms.field());
ArrayColumn<double> ref_dir(_field, MSField::columnName(MSFieldEnums::PHASE_DIR));
Array<double> dir = ref_dir(0);
double *ph = dir.data();
double ra0 = ph[0];
double dec0 = ph[1];

My understanding is that in the MS, the RA and DEC is stored in radians. Now, in the later part of the script, I see a snippet where the RA and DEC are being converted to rectangular coordinates. But they are also multiplied by rpd_c() which if I understand correctly is a function in CSPICE package that converts angles from degrees to radians. radrec_c(1.0, ra0 * rpd_c(), dec0 * rpd_c(), v2000);

I assume the radrec_c function expects its angular inputs (RA and Dec) to be in radians, so if they are already in the correct unit, we can directly pass them to the function without any conversion.

Could you kindly look into this and make the necessary changes? Thanks in advance!

SarodYatawatta commented 2 months ago

Fixed