pycbc
pycbc copied to clipboard
not use inverse_spectrum_truncation in TDI-2 PE
This PR fixs the Welch TDI-2 PSD in PE. For unknown reasons, the Welch TDI-2 PSD after inverse_spectrum_truncation will look
like this (but TDI-1.5 still looks good):
So I decided to remove
psd-inverse-length = 267840 for now.
This is a: bug fix
This change affects: inference
This change changes: documentation
Motivation
Adding support for the upcoming TDI-2 waveform plugin.
Contents
Remove psd-inverse-length = 267840 for now.
Links to any issues or associated PRs
https://github.com/gwastro/BBHX-waveform-model/pull/5
Testing performed
I removed the psd-inverse-length = 267840, then plotted the Welch PSD, it matches the analytical one:
I also tested the TDI-2 PSD with TDI-2 waveform plugin PR, looks good:
- [x] The author of this pull request confirms they will adhere to the code of conduct
Recall that it is used to condition the whitening filter to prevent wraparound corruption. That still needs to be done, so you may need to look a bit deeper.
@WuShichao Does this https://github.com/gwastro/pycbc/pull/4225 fix the issue?
@WuShichao Does this #4225 fix the issue?
@WuShichao This is definitely worth a try. It makes some sense since the current truncation is somewhat over simple.
The lines in the PSD tending to 0 definitely cause an issue when trying to whiten.
@spxiwh Yes, I think if the truncation is optimal you should get away with a broadening of the (inverse) lines rather than what Shichao is seeing. There's definitely no 'perfect' solution here (at least not without the idealized world of infinite timeseries), but clearly it's not great. I suspect you hit the right track though in looking how the inverse spectrum is truncated (possible it needs to be allowed to be a bit longer too, but we'll see).
In some sense the perfect solution is simply to not modify the noise definition, but rather put it all in the waveform model. That should have less sharp features as most of this cancels with similar corrections that exist on the waveform side (though there's an additional phase term in the signal side if I remember right).
Something to discuss at the next call!
@WuShichao Does this #4225 fix the issue?
@ahnitz @spxiwh Thanks a lot! hann fixes my issue, I have read all the comments in that PR draft, it seems that PR is still waiting for more tests for PyCBC Live, so I decided to add invpsd-trunc-method = hann for now. Below are some plots in my test:
After setting invpsd-trunc-method = hann:
Great that this works ... I guess a reminder to me to get #4225 merged. As a config file change only now, this all looks good to me!