gwastro / BBHX-waveform-model

GNU General Public License v3.0
1 stars 3 forks source link

add TDI-2 support #5

Closed WuShichao closed 2 months ago

WuShichao commented 3 months ago

@ahnitz @spxiwh This PR adds TDI-2 conversion to the BBHx waveform. Original BBHx's response is TDI-1.5, I'm using those equitations to convert TDI-1.5 to TDI-2.0: image Those are for the X/Y/Z channels, but the A/E/T channels are just linear combinations of X/Y/Z, so they still follow these.

WuShichao commented 2 months ago

@mj-will Alex wants you to review this TDI-2.0 PR. Thanks a lot!

mj-will commented 2 months ago

@mj-will Alex wants you to review this TDI-2.0 PR. Thanks a lot!

Sure thing, I'm at a conference this week, so I most likely won't get to this until next week. Hope that's okay

mj-will commented 2 months ago

I've added some initial comments but still need to go over things more thoroughly.

WuShichao commented 2 months ago

I've gone over the rest of this and it looks fine to me (except for some very minor comments). @WuShichao do you have any results produced using this that you could link to?

@mj-will Yes, the TDI-2 PE test is here https://github.com/gwastro/pycbc/pull/4738

mj-will commented 2 months ago

@WuShichao given we've got the CI now, could you add a TDI 1.5 and 2.0 to the existing test? You should be able to use a parametrize mark to add it so the test gets run on both TDIs and reference frames.