Merge branch 'master' into 'master'

fix td waveform epoch

See merge request !736

bilby/gw/source.py

@@ -396,6 +396,13 @@ def _base_lal_cbc_fd_waveform(
     h_plus *= frequency_bounds
     h_cross *= frequency_bounds
 
+    if wf_func == lalsim_SimInspiralFD:
+        dt = 1. / delta_frequency - (hplus.epoch.gpsSeconds + hplus.epoch.gpsNanoSeconds * 1e-9)
+        h_plus *= np.exp(
+            -1j * 2 * np.pi * dt * frequency_array)
+        h_cross *= np.exp(
+            -1j * 2 * np.pi * dt * frequency_array)
+
     return dict(plus=h_plus, cross=h_cross)
 
 

[Roberto Cotesta]

It has to be dt = 1. / delta_frequency + (hplus.epoch.gpsSeconds + hplus.epoch.gpsNanoSeconds * 1e-9) or dt = 1. / delta_frequency - np.abs(hplus.epoch.gpsSeconds + hplus.epoch.gpsNanoSeconds * 1e-9) as it was before I believe

[Roberto Cotesta]

@rory-smith @gregory.ashton

[Roberto Cotesta]

Just to give more informations, myself and Serguei did the test here on a branch with this modification: dt = (T - abs(hplus.epoch.gpsSeconds+hplus.epoch.gpsNanoSeconds*1e-9)). Epoch is a quantity always defined as negative because is the first element of the time array assuming that t = 0 is the peak of the 22 mode (for only 22 mode waveforms) and an invariant amplitude for multipolar waveforms. The peak of this invariant amplitude is anyway close to the peak of the 22 mode. So either you use the abs or you change the - sign with +. I've just made a modification to my branch of bilby to test this. I'll show later today a comparison on a run.

[Rory Smith]

Oh no! Does this affect any of the on going runs?

[Roberto Cotesta]

Yes, see here. I think it can be fixed in postprocessing, but I'd rather have it correct.

[Roberto Cotesta]

@rory-smith @gregory.ashton The patch that I made produce the correct result, I guess you guys can correct this typo in the merge request. As reference with my patch (i.e. dt = 1. / delta_frequency + (hplus.epoch.gpsSeconds + hplus.epoch.gpsNanoSeconds * 1e-9)) I can match the tc in a @serguei.ossokine run. That run matched LI results (see here). I apologize that I didn't bother to compare directly the run with my patch with the LI ones. I'm lazy and it's quite late here :) In principle I can open a merge request myself, in practice I believe that it's easier if you fix this since it's just a sign. I hope that what I've just written is understandable