Skip to content

WIP: XLALAdd[Type]TimeSeries(): add sample-alignment safety check

David Keitel requested to merge david-keitel/lalsuite:XLALAddTimeSeries-sample-alignment-check into master

Description

This is another small change split off from Reinhard Prix's suggestions on #275 (closed), but this one is a bit non-trivial since the affected function is used extensively throughout LALSuite. The addressed problem, in Reinhard's words:

- when adding timeseries, make sure the corresponding time-samples
  align (within 10eps), otherwise throw an error
- the current behavior of silently rounding to the nearest bin
  is scientifically unsafe

API Changes and Justification

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

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

This added safety check sounds reasonable to me. There may be codes relying on the lenient behaviour, but at least the patch won't silently change anything, and when people see the clear error, they should be able to adapt. Still, let's see if there are concerns.

I attach a small test script demonstrating the effect before:

Base ts: t0=1000000000.00, duration=10.00, 10 bins...
Adding ts of ones with t0=1000000001.00, duration=2.00, 2 bins...
[ 0.  1.  1.  0.  0.  0.  0.  0.  0.  0.]
Adding ts of ones with t0=1000000001.00, duration=3.20, 4 bins...
[ 0.  2.  2.  1.  1.  0.  0.  0.  0.  0.]
Adding ts of ones with t0=1000000001.42, duration=4.00, 4 bins...
[ 0.  3.  3.  2.  2.  0.  0.  0.  0.  0.]

and after:

Base ts: t0=1000000000.00, duration=10.00, 10 bins...
Adding ts of ones with t0=1000000001.00, duration=2.00, 2 bins...
[0. 1. 1. 0. 0. 0. 0. 0. 0. 0.]
Adding ts of ones with t0=1000000001.00, duration=3.20, 4 bins...
[0. 2. 2. 1. 1. 0. 0. 0. 0. 0.]
Adding ts of ones with t0=1000000001.42, duration=4.00, 4 bins...
XLALAddREAL4TimeSeries(): incompatible start-times, bins misaligned by 0.42 deltaT
XLAL Error - XLALAddREAL4TimeSeries (TimeSeries_source.c:161): Invalid data
Invalid data

Assigning @jolien-creighton; cc @karl-wette.

[Edited test script to remove confusing 4th case where I had rounded up the duration myself.]

Edited by Ghost User

Merge request reports