-
Notifications
You must be signed in to change notification settings - Fork 20
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
Adding tidal waveforms #79
Conversation
Update typing information
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.
Is there a reason why you would use another waveform model as the reference waveform?
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.
The NRTidalv2 waveforms have a tapering window for the amplitude such that the amplitude reaches zero at around 1.2 times the merger frequency, which depends on the parameters of the waveform. We observed that this breaks the relative binning in case the merger frequency of the reference waveform is lower than of the waveform of sampled parameters, since the reference waveform enters the denominator of the relative binning approximation and will cause it to blow up. We fixed this and improved the stability by removing this taper from the reference waveform while still using the taper for sampled waveforms. Our pp plot showed we still get decent and consistent results, but this therefore requires a bit more flexibility in the likelihood file.
minor bug fix
This pull request tries to merge the new ripple waveforms TaylorF2 and IMRPhenomD_NRTidalv2 waveforms into the jim main branch. I'd like to give a few more comments about this:
For source code: besides adding a way to interface with ripple to get the waveforms, I have made small changes to likelihood.py to enable users to use a different waveform as a reference waveform in the heterodyned likelihood. This is necessary for the IMRPhenomD_NRTidalv2 waveform, where we want to remove the Planck taper from the waveform as explained by our paper.
For the examples: these are not finalized yet, for the following reasons: