Stricter Gating for the TDCFs
This MR will apply stricter gating to the TDCFs. We encountered a problem using gstlal-calibration-1.5.7 where the TDCFs were not gated well enough during sweeps, or at any time when the lines are turned off. When the lines are off, the coherence calculation that we use to gate does not respond until the entire running median is filled with corrupted TDCF values. This propagated the the line subtraction TFs, which led to significant failure during and shortly after sweeps. See this plot (around 18:30 - 19:00 UTC) for an example. Also, see kappa_C at that time for an example of what caused it. This did not affect the production pipeline for a rather peculiar reason: the un-medianed TDCFs are computed as nan's instead of very large or very small values in gstlal-calibration-1.5.4. Although this behavior is not necessarily desired, it caused the running median to reject those values and prevented the line subtraction problem seen in the above linked plot.
Two new gating methods can be (optionally) applied to the TDCF calculation with this MR. First, we can gate with the line amplitude channels (with names like {IFO}:CAL-CS_LINE_[1-3]_OSC_CLKGAIN
or CAL-PCAL[X/Y]_PCALOSC[1-9]_OSC_SINGAIN
). Second, we can also gate with the excitation channels themselves. Here is the raitionale for adding both of these rather than just one:
- If we encounter a situation where the pipeline cannot access an excitation channel for some time (due to, e.g., a raw data dropout), only the excitation channel itself can tell us that.
- When another type of injection is occurring (e.g., a Pcal sweep), the injection channel will not carry zeros, so the corresponding amplitude channel is needed in that case.
I have verified that both methods work. Below is a plot showing kappa_C and f_cc after the fix. Note that the excursion shown in the above plot of kappa_C is removed, as expected.
The next plot shows that I can reproduce the error offline using gstlal-1.5.7:
This last plot shows that the line subtraction works as expected after the fix. I used both gating methods to make this plot.
The sweep, as well as a broadband injection, can be seen in this plot. We make no effort to subtract these.
Merge request reports
Activity
requested review from @madeline-wade and @jameson.rollins
assigned to @aaron-viets
The CI passed: aaron-viets/gstlal-calibration@a0aba28d
@madeline-wade I am not sure why that pipeline is not working. I think something is wrong with the CI configs specifically in relation to merge requests. Here is the most recent CI pipeline from the branch being merged. So, the CI should pass when it is merged. The same thing happened in the last MR. I think it is a result of choosing to make the CI tests manual for pushes, but automatic for merge requests. The manual CI tests are working as expected, but I think something has to be changed for the MR pipeline to do what we want. I didn't realize this would not work until the MR was made. If you just want the "failed pipeline" message to not happen, the quick fix would be to eliminate the automatic CI tests for MRs, and only have the manual tests for now.
@madeline-wade If you are looking through the diff, you may notice that I also removed the deprecated SRC Pcal line stuff, which we have not used since O2, when there was a 7.93-Hz line at H1. Keeping that seemed unnecessary and would have made this MR more complicated.
- Resolved by Madeline Wade
@aaron-viets Would this operate by gating with both options at the same time or do we have to choose one or another in the config?
- Resolved by Madeline Wade
@aaron-viets @jameson.rollins I am happy with this MR.
reset approvals from @madeline-wade by pushing to the branch
@madeline-wade @jameson.rollins One more thing before I merge: should I remove the shebangs at the beginning of gstlal_compute_strain and other pipelines in the same directory? This line:
#!/usr/bin/env python3
I seem to recall that we were supposed to remove those.
@aaron-viets Yes there are better ways to handle the interpreter, but let's not bother with that right now, as there we'll have to figure out another way to wrap the code for the application, and that will take some extra thought.
mentioned in commit 27c9fc13