-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add TDI-2 support #5
Conversation
@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 |
I've added some initial comments but still need to go over things more thoroughly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 gwastro/pycbc#4738 |
@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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @WuShichao, this now looks good to me!
@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:
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.