pycbc icon indicating copy to clipboard operation
pycbc copied to clipboard

[WIP] new waveform inteface implimentation: awaiting review and lalsuite release

Open bhooshan-gadre opened this issue 2 years ago • 4 comments

This PR adds cleaner implementation for polarizations and waveform modes. This supersedes #4194

Tested with [email protected]:waveforms/reviews/lalsuite.git hash 1da1a023b94b805ab238b863f922eb42b97d1374

bhooshan-gadre avatar Mar 20 '23 16:03 bhooshan-gadre

@bhooshan-gadre once you have this working on your end, I think you should revisit just having this as code within gwsignal to advertise itself as a pycbc plugin. We already have an interface for this and would be the more natural location. Afterall, pycbc's goal it to provide a generic waveform interface that anyone can build a plugin for.

ahnitz avatar Mar 20 '23 17:03 ahnitz

@bhooshan-gadre This would also hopefully make it so any change isn't a nightmare for forward / backwards compatibility.

ahnitz avatar Mar 20 '23 17:03 ahnitz

This now works with the lalsimulation version 5.1.0.1 of released lalsuite

  1. gwsingal_utils converts GR params from pycbc to gwsignal-lalsimulation.
  2. This also allows one to use SEOBNRv5 iff one installs pyseobnr using pip install pyseobnr woth other lal waveforms
  3. Now there is a possibility of getting td waveform conditioning. Currently, it is set to off(=0).
  4. Also, the gws waveform generator also converts td approximants to fd and vise-versa. I have not deliberately changed anything to allow td <--> fd conversions.
  5. GWS also has a waveform mode generator. I have added the same for the waveform modes. Not yet tested if it always works and if pycbc and gws conventions are now consistent though I tried a bit!

bhooshan-gadre avatar Jul 25 '23 10:07 bhooshan-gadre

@bhooshan-gadre Thanks for doing this.

The code you've put together is essentially what is needed for the plugin interface I noted above already which is great. There is an example linked below and a few additional interfaces (not in the below doc, but similar implementation) provide things like length estimators where existing, etc. I think we can essentially move this code to a file in gwsim and add in some metadata to the setup.py/cfg. If we need to extend this to help enable additional features for e.g. the higher mode interface, I think we can do that in pycbc and I'd be happy to help out with that.

https://pycbc.org/pycbc/latest/html/waveform_plugin.html

The pycbc waveform package general idea has been to act as a common easy-to-use python frontend for all gw waveform generation that people can build upon. To that end, we've had a plugin interface for a long time now for waveform libraries to use. There are plugins for several custom waveforms, teobnr, seobnre, gwsurrogate, etc already. While lalsimulation has been an exception and hardcoded in the past, I think this large of a change is the right time to make this consistent rather than incur additional technical debt.

On a separate note, I wouldn't worry about td/fd conversion in gwsim, we already handle this automatically in pycbc (better in a many ways than the version in lalsimulation was). The only thing needed to enable it is to have a length estimator, so I think that's the important thing to keep in mind.

@ahnitz So, your suggestion is to do away with lalsimulation via _lalsim_(f/t)d_waveform and interface everything from pycbc waveform interface (gwsim) instead. Is that so?

bhooshan-gadre avatar Jul 26 '23 09:07 bhooshan-gadre