PredictFstat maxStartTime fix (new alternative option: --duration)
Description
-
First I noticed an omission in the test script: since the
--maxStartTime
option expects the starting timestamp of the last SFT to cover, not the end time, this test script was actually calling PFS the wrong way. This slipped through because of an over-generous 1% tolerance - actually the 3 runs which all useassumeSqrtSX
and just different ways of selecting timestamps should be exactly equivalent, and indeed they are after the fix; but I've now set it to 1e-6 just to still be safe against cosmic rays. ;) -
Then, as described in #370 (closed), I realised that this behaviour of
--maxStartTime
when no --DataFiles are given is actually inconsistent with the opposite case and the underlying half-open-interval policy defined by XLALCWGPSinRange. So this extended MR now introduces a new--duration
option to be used when when no --DataFiles are given, which should behave consistently with both human expectations and the underlying convention, while disallowing--maxStartTime
in this situation so that no old commandlines silently change behaviour.
API Changes and Justification
Backwards Compatible Changes
As usual with lalapps changes, these are not technically API changes. A new commandline option is added and some previously supported combinations of options now throw errors, though.
-
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
To demonstrate the first (purely test script) fix, the relevant part of the test log after tightening the tolerances, but still using endTime
:
SemiAnalyticF: 2F_SA = 51093.8
PredictFstat{assumeSqrtSX}: 2F_PF0 = 51093.7
PredictFstat{NoiseWeights}: 2F_PF1 = 47034.4
PredictFstat{timestamps,assumeSqrtSX}: 2F_PF2 = 51093.7
PredictFstat{minStartTime+maxStartTime,assumeSqrtSX}: 2F_PF2 = 51283.8
Relative deviation 2F_PF{assumeSqrtSX} wrt 2F_SA = 0.0002% (tolerance = 1%) ==> OK.
Relative deviation 2F_PF{NoiseWeights} wrt 2F_PF{assumeSqrtSX} = 8.2735% (tolerance = 20%) ==> OK.
Relative deviation 2F_PF{timestamps,assumeSqrtSX} wrt 2F_PF{assumeSqrtSX} = 0.0000% (tolerance = 0.0001%) ==> OK.
Relative deviation 2F_PF{minStartTime+maxStartTime,assumeSqrtSX} wrt 2F_PF{assumeSqrtSX} = 0.3714% (tolerance = 0.0001%) ==> FAILED.
after correctly using maxStartTime
:
SemiAnalyticF: 2F_SA = 51093.8
PredictFstat{assumeSqrtSX}: 2F_PF0 = 51093.7
PredictFstat{NoiseWeights}: 2F_PF1 = 47034.4
PredictFstat{timestamps,assumeSqrtSX}: 2F_PF2 = 51093.7
PredictFstat{minStartTime+maxStartTime,assumeSqrtSX}: 2F_PF2 = 51093.7
Relative deviation 2F_PF{assumeSqrtSX} wrt 2F_SA = 0.0002% (tolerance = 1%) ==> OK.
Relative deviation 2F_PF{NoiseWeights} wrt 2F_PF{assumeSqrtSX} = 8.2735% (tolerance = 20%) ==> OK.
Relative deviation 2F_PF{timestamps,assumeSqrtSX} wrt 2F_PF{assumeSqrtSX} = 0.0000% (tolerance = 0.0001%) ==> OK.
Relative deviation 2F_PF{minStartTime+maxStartTime,assumeSqrtSX} wrt 2F_PF{assumeSqrtSX} = 0.0000% (tolerance = 0.0001%) ==> OK.
To demonstrate the fix of #370 (closed), repeating the example commandlines from over there will now produce an error from the last two, but changing them to use --duration
instead now produces
$ lalapps_PredictFstat --outputFstat=$tmpPFS --h0=1 --cosi=-0.3 --psi=0.6 --Alpha=2 --Delta=-0.5 --assumeSqrtSX=0.5 --IFOs=H1 --minStartTime=700000000 --duration=1800 && grep Loaded $tmpPFS
228.4
%% Loaded SFTs: [ LHO_4k:1 ]
$ lalapps_PredictFstat --outputFstat=$tmpPFS --h0=1 --cosi=-0.3 --psi=0.6 --Alpha=2 --Delta=-0.5 --assumeSqrtSX=0.5 --IFOs=H1 --minStartTime=700000000 --duration=3600 && grep Loaded $tmpPFS
421.9
%% Loaded SFTs: [ LHO_4k:2 ]
which is consistent with the other results.