[Difx-users] pcal_200213 bug in difx2fits

[harro verkouter]

Hi all,

Reading that patch I'm not convinced that randomly adding explicit "(int)"
conversions is the proper fix.
Looking at the difx_datastream.c code the intent of the "toneFreq0"
variable is to hold the first integer multiple of the PCal frequency that
falls within the actual datastream (subband, channel, observed band, chunk
of sky frequency described).

My suggestion would be something along the lines of:

     toneFreq0 = (int)(( loFreq + dd->phaseCalIntervalMHz ) /
dd->phaseCalIntervalMHz ) * dd->phaseCalIntervalMHz;

Two birds with one stone:
1) the special case "if(toneFreq0) <= loFreq" can be removed as it will be
dealt with automatically
2) only the first part of the computation needs to be converted to int
(i.e. the rounding to an integer number of pcal tones); the multiplication
will produce a double again, but the assignment to the integer "toneFreq0"
will automatically truncate it again.

Cheers,
h


On Thu, 13 Feb 2020 at 18:14, Leonid Petrov via Difx-users <
difx-users at listmgr.nrao.edu> wrote:

> Dear DiFXers,
>
>    pcal_200213 bug in difx2fits was found and fixed:
>
> Synopsis:
>
>    Phase calibration frequencies in FITS-IDI are not commensurate to the
> phase
> cal tone spacings and some frequencies are NaN if for the lower
> frequency
> of a given IF is no commensurate to the phase cal tone spacings. Was
> found
> in VLBI experiment VT9259 with IF#1 LO Frequency 3448.4 MHz and pcal
> tone
> spacings 5 MHz.
>
> Severity:
>
>    Serious
>
> Reason:
>
>    A trivial coding mistake
>
> Patch:
>
> --- libraries/difxio/difxio/difx_datastream.c~  2019-04-13
> 19:05:23.600324077 -0400
> +++ libraries/difxio/difxio/difx_datastream.c   2020-02-13
> 11:28:46.109375671 -0500
> @@ -342,7 +342,7 @@
>
>         if(df->sideband == 'U')
>         {
> -               toneFreq0 = (((int)(loFreq)) / dd->phaseCalIntervalMHz) *
> dd->phaseCalIntervalMHz;
> +               toneFreq0 = ( (int) loFreq  / (int)
> dd->phaseCalIntervalMHz ) * (int)
> dd->phaseCalIntervalMHz;
>                 if(toneFreq0 <= loFreq)
>                 {
>                         toneFreq0 += dd->phaseCalIntervalMHz;
> @@ -350,7 +350,7 @@
>         }
>         else
>         {
> -               toneFreq0 = (((int)(loFreq)) / dd->phaseCalIntervalMHz) *
> dd->phaseCalIntervalMHz;
> +               toneFreq0 = ( (int) loFreq  / (int)
> dd->phaseCalIntervalMHz ) * (int)
> dd->phaseCalIntervalMHz;
>                 if(toneFreq0 == loFreq)
>                 {
>                         toneFreq0 -= dd->phaseCalIntervalMHz;
>
>
> Leonid
> 2020.02.13_12:13:06
>
> _______________________________________________
> Difx-users mailing list
> Difx-users at listmgr.nrao.edu
> https://listmgr.nrao.edu/mailman/listinfo/difx-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listmgr.nrao.edu/pipermail/difx-users/attachments/20200213/1e2ed7f2/attachment.html>