Skip to content

Clean up the overlap+window parameters misused in the code

Arianna Renzini requested to merge fixing-bias-factors into master

This MR sets out to resolve issues #78 (closed) and #75 (closed) . See these issues for more details.

After taking a closer look, I have come up with the proposal here:

  • baseline : fix overlap+window used for Welch PSD estimation, keep overlap+window used for CSD estimation as parameters. This then also fixes these automatically for the pipeline. This is easy to then update, once some testing has taken place, see comments in the code.
  • postprocessing : expects information about Welch PSD estimation: overlap_factor and window_fft_dict. These are not in general the same as those set currently in the baseline, which refer primarily to the CSD estimation. This is why it's a bit hairy to fix this. Extra parameters need to be added to Baseline to support this.
  • delta_sigma_cut : same as postprocessing. Extra parameters need to be added to Baseline to support the changes fully.
  • util : has been updated to (hopefully!) support non-overlapping, non-Hann Welch PSD estimations. This is reflected in window calculations, rho calculation, and bias factor calculation. This still needs to be tested - a set of reference bias factors / rho factors / etc. to compare with would be useful.

In doing this, I've also found inconsistencies when trying to use an N_avg_segs parameter different than 2. So I have now tentatively fixed this by setting a new property in the baseline, csd_segment_offset, which depends also on how many segments have been used to estimate the PSD (the first segment in the analysis will be necessarily shifted depending on how many segments we use to calculate the average PSD...). This begs the question though, is the calculation of the window factors, etc, correct, or does it also assume an N_avg_segs=2? E.g., in line 139 of postprocessing, should we be passing a larger number as N which includes the N_avg_segs ? Perhaps @patrick-meyers may have thought of this already.

I've verified that even if there are some small mistakes in the implementation of window_factors, these are so small they do not change our results in any appreciable way, when using default parameters. I think it would be good for this MR to go through so that more people can test the code with non-default parameters and weed out the bugs.

Edited by Arianna Renzini

Merge request reports