FEAT: add gwsignal waveform generator

[ColmTalbot]

@raffienficiaud this is a quick version of how I had envisioned an extension to the gwsignal support. It doesn't require explicit enumeration of all of the different kinds of source models, users can just do something like

import bilby

priors = bilby.gw.prior.BBHPriorDict()
priors["eccentricity"] = bilby.core.prior.Uniform(0, 1)
priors["mean_per_ano"] = 0.0
del priors["a_1"], priors["a_2"], priors["tilt_1"], priors["tilt_2"], priors["phi_jl"], priors["phi_12"]

wfg = bilby.gw.waveform_generator.GWSignalWaveformGenerator(
    waveform_arguments=dict(waveform_approximant="EccentricFD"),
    duration=4,
    sampling_frequency=1024,
    eccentric=True,
    spinning=False,
)

wfg.frequency_domain_strain(priors.sample())

I'm sure there are improvements that could be made to the interface, but I'm hopeful that something like this can be fairly transparent to the underlying gwsignal capabilities and ideally not need a lot of modification as more features are added to that. Since you have more experience with gwsignal I'd be glad to hear any thoughts.

Also @adivijaykumar @AntoniRamosBuades and @mj-will

[mj-will]

Is there a reason to use 0/1 instead of True/False?

[ColmTalbot]

This is copied from the existing function (https://github.com/bilby-dev/bilby/blob/main/bilby/gw/source.py#L149). I think this is more of a question for the gwsignal devs, I think @AntoniRamosBuades added this.

[AntoniRamosBuades]

The use of 0/1 was added in the first stage of the gwsignal development (see this notebook for more details on the conditioning in gwsignal). Unfortunately, I do not know why 0/1 was chosen, instead of False/True

[mj-will]

Thanks for doing things @ColmTalbot, it looks like a good start to me! I had a quick look and added some comments.

[lorenzopompili00]

Hi @ColmTalbot I also really like this idea!

Just want to mention that for bilby_tgr we also have a gwsignal_binary_black_hole function, which is a almost copy of the bilby one with just some extra lines here. To improve maintainability and reduce code duplication, it would be great to have an implementation that allows the waveform generator for bilby_tgr to be a subclass of the bilby one, without duplicating too much code as is done right now. I still need to look at this more closely, but I wanted to mention this as something to consider.

[raffienficiaud]

Thank you for the PR @ColmTalbot . I am all fine with any changes you make.
I have a questions about this interface: it relies on the fact that we can instanciate the GWSignalWaveformGenerator class with parameters: is this compatible with the way WF generators can be instanciated in the configuration of bilby_pipe?

[ColmTalbot]

Thank you for the PR @ColmTalbot . I am all fine with any changes you make. I have a questions about this interface: it relies on the fact that we can instanciate the GWSignalWaveformGenerator class with parameters: is this compatible with the way WF generators can be instanciated in the configuration of bilby_pipe?

That's a good question, it looks like it currently isn't possible. I see two options:

• add this as an option in a future release of bilby_pipe
• make the options here something that is passed through the waveform_kwargs

[raffienficiaud]

• add this as an option in a future release of bilby_pipe

I guess this is the way to go, but I don't know if we can make this ready by the freeze end of March (according to your email).

• make the options here something that is passed through the waveform_kwargs

For this, we would need a wrapper class that acts as a "factory" for the implementation suggested in the PR.

I don't know how hard is it to get the change into bilby_pipe, and I can give a hand. However, I would like in advance to check with the devs if end of March is something feasible from their perspective.