Fix bug in DCS filtering related to delays

This MR fixes issues with actuation stage calculations of transfer functions for FIR filtering. There were missing OMC to SUS delays not captured by the methods, and an incorrect dividing out when using the correct function. Likely the rationale for the latter was to preserve the phase for CI calculations.

This MR also include a change to the CI unit test DCS filter reference file: the old file was most likely created using the new pyDARM (not the O3 pyDARM version) and so it had an error already encoded into the reference DCS filter. I created a new file and saved only the actuation_tst filter to that file so the size of the file is much smaller than before.

I also made some small code cleanup adjustments that does not affect the functionality of the code.

Closes #271 (closed)

added Code cleanup bugfix labels

assigned to @evan-goetz

added 1 commit

• 172d4ebe - Fix bug in DCS filtering related to delays

Compare with previous version

requested review from @aaron-viets, @avanivikrambhai.patel, and @joseph-betzwieser

added 1 commit

• 5df842d5 - Fix bug in DCS filtering related to delays

Compare with previous version

marked this merge request as ready

Below are some plots showing comparisons of the frequency response of the filters in the unified (DCS) calibration pipeline to the frequency-domain models they represent. The frequency response of the filters is inferred by taking real data (e.g., DARM_ERR and DCS-CALIB_STRAIN) from before and after the application and computing the TF. Here is a plot for the TST filter from before the fix that is implemented in this MR:

To show that the phase error shown in the previous plot is an error in the filter rather than in the model, see the corresponding plot for the response function, which shows evidence of a relative timing error between sensing and actuation:

After the fix, both plots look as expected:

Here is one last plot showing that the filters produced using this MR can reproduce data that agrees with C00 data. This is a TF between h(t) produced using the filters produced with this MR and h(t) from C00 data:

These show that the timing bug is fixed.

@evan-goetz Since this seems to have been resolved by using stage_super_actuator() instead of compute_actuation_single_stage(), does that mean that there is still a timing bug in compute_actuation_single_stage(), or is there an expected one-clock-cycle difference? Can we remove compute_actuation_single_stage(), or are there other places where it is used?

approved this merge request

resolved all threads

added 1 commit

• 283a73b9 - Fix bug in DCS filtering related to delays

Compare with previous version

reset approvals from @aaron-viets by pushing to the branch

changed the description

@aaron-viets @joseph-betzwieser @avanivikrambhai.patel I have now updated this MR so that the CI passes. I replaced the DCS filter file used as a reference for the unit tests with a corrected version. Note that this change was made because Aaron's tests above show that this MR is producing the correct result that is consistent with the DARM model transfer function and the CALCS+GDS filtering.

Please give any final review and approve when you are able. Thanks!

approved this merge request

@evan-goetz is this ready to go now? should we push this into production so that it's in place for our next set of reports?

approved this merge request

resolved all threads

merged

mentioned in commit c3486347