Resolve "Use integrator/differentiator with vanishing group delay"

Closes #38 (closed)

• Use cumulative trapezoidal integration to compute timer offsets

changed milestone to %v1.0.1

added bug label

added 1 commit

• fd93efad - Replace `cumsum` by `cumulative_trapezoid`

Compare with previous version

Hi @j2b.bayle,

this should work now. I ran the following test: I compared the two-point derivative (np.gradient) of instrument.local_timer_deviations to the actual clock drift which is given by instrument.clock_noise_offsets + instrument.clock_noise_fluctuations. The CSD of the two shows if there is any residual group delay present.

Below I show the result for the current master and for this branch, in blue the amplitude (left y-axis) and in orange the phase (right y-axis) of the CSD. We can clearly see that there is no group delay present anymore.

The code for reference:

import lisainstrument
instrument = lisainstrument.instrument.Instrument(size=1e4)
instrument.simulate()

clock_drift_true = instrument.clock_noise_offsets['1'] + instrument.clock_noise_fluctuations['1']
clock_drift_derived = np.gradient(instrument.local_timer_deviations['1'], instrument.physics_dt)

freq, csd = si.csd(clock_drift_derived, clock_drift_true, fs=instrument.physics_fs, nperseg=1e3)
plt.plot(freq, np.abs(csd), c='tab:blue', label='CSD amplitude')
plt.twinx()
plt.plot(freq, np.angle(csd), c='tab:orange', label='CSD phase')
plt.plot(freq, 0* freq, c='black', ls='--')

requested review from @j2b.bayle

That's awesome. Love Numpy and Scipy, there's always what we need . Is that still a draft @mastaa?

marked this merge request as ready

I did not do any "integration testing" yet... How can we test if everything works as before?

Unfortunately there are no integration tests yet, so you might want to run a simulation on master, and one on this branch, and then compare. But yeah, at some point we should set up tests!

changed the description

I've run a simple simulation with usual parameters, and it seems to be good! Merging!

added 4 commits

• fd93efad...e8bbe207 - 3 commits from branch master
• 71ac4382 - Replace `cumsum` by `cumulative_trapezoid`

Compare with previous version

enabled an automatic merge when the pipeline for 71ac4382 succeeds

merged

mentioned in commit 487a22cd