Skip to content

fix the truncation error in LIGOTimeGPS set from XLALGPSSetREAL8

Wanting Niu requested to merge (removed):master into master

Description

A "bug" fix explained in this issue: #683 (closed). In the existing calculation of gpsNanoSecond in XLALGPSSetREAL8 (in lal/lib/date/XLALTime.c), there is a truncation error caused by subtracting a INT4 variable from a REAL8 variable in this line: https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lal/lib/date/XLALTime.c#L76.

For example, if we input a REAL8 t=1234567890.1234 into XLALGPSSetREAL8, instead of outputting the INT4 gpsSecond=1234567890 and INT4 gpsNanoSecond=123400000, we will get the outputted nanosecond part to be gpsNanoSecond=123399973. This difference is an unnecessary inaccuracy produced by a truncation error in C. We should fix this issue to maintain the precision of LIGOTimeGPS.

API Changes and Justification

Backwards Compatible Changes

  • This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions
  • This change adds new classes/functions/structs/types to a public C header file or Python module

Backwards Incompatible Changes

  • This change modifies an existing class/function/struct/type definition in a public C header file or Python module
  • This change removes an existing class/function/struct/type from a public C header file or Python module

If any of the Backwards Incompatible check boxes are ticked please provide a justification why this change is necessary and why it needs to be done in a backwards incompatible way.

Review Status

It would be great if @jolien-creighton or @kipp.cannon can have some review on it. Here is some local test I did:

before the change:

>>> t = 1234567890.10001
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
100009918
>>> t = 1234567890.1234
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123399973
>>> t = 1234567890.10001
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
100009918
>>> t = 1234567890.123456
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123456001
>>> t = 1234567890.12345
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123450041

after the change:

>>> t = 1234567890.10001
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
100010000
>>> t = 1234567890.1234
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123400000
>>> t = 1234567890.10001
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
100010000
>>> t = 1234567890.123456
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123456000
>>> t = 1234567890.12345
>>> time = lal.LIGOTimeGPS(t)
>>> time.gpsNanoSeconds
123450000
Edited by Wanting Niu

Merge request reports