pycbc icon indicating copy to clipboard operation
pycbc copied to clipboard

not use inverse_spectrum_truncation in TDI-2 PE

Open WuShichao opened this issue 1 year ago • 6 comments

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): image 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: image I also tested the TDI-2 PSD with TDI-2 waveform plugin PR, looks good: image

  • [x] The author of this pull request confirms they will adhere to the code of conduct

WuShichao avatar May 04 '24 21:05 WuShichao

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.

ahnitz avatar May 04 '24 22:05 ahnitz

@WuShichao Does this https://github.com/gwastro/pycbc/pull/4225 fix the issue?

spxiwh avatar May 05 '24 19:05 spxiwh

@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.

ahnitz avatar May 05 '24 19:05 ahnitz

The lines in the PSD tending to 0 definitely cause an issue when trying to whiten.

spxiwh avatar May 05 '24 19:05 spxiwh

@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).

ahnitz avatar May 05 '24 20:05 ahnitz

Something to discuss at the next call!

spxiwh avatar May 05 '24 20:05 spxiwh

@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: image image

WuShichao avatar May 06 '24 13:05 WuShichao

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!

spxiwh avatar May 06 '24 19:05 spxiwh