Update sources to be functions

An idea inspired by discussion with @paul-lasky and @moritz.huebner.

Users should be able to input a function alone for the source and shouldn't have to worry about subclassing something. This enables this for the current implementation, and could easily be modified for the changes suggested in !13 (merged). Additionally I think it solves (my) issue with !13 (merged) - namely that user defined sources are subject to errors.

Key changes

1. The Source class is replaced by a WaveformGenerator class

This is really just a more accurate name for what is currently implemented. As noted by Moritz, semantically the Source (i.e. a BBH) doesn't care about the frequencies or time-series it is computed at.

2. The WaveformGenerator take a function source_model which returns the frequency-domain strain. It then does the introspection to work out what parameters the source_model needs and defaults them to None.

Currently, it still uses "the pass in the parameters" when calculating the frequency domain strain approach. But can easily be modified to fit with !13 (merged) . We can also add functions to set the values (if for example one wants to inject a signal).

Example

Here is psuedo-code example of how it will eventually work (the BasicTutorial.py currently does work but is missing some of the functionality described here).

def BBH(frequency_array, mass_1, mass_2, luminosity_distance, spin_1, spin_2,
        iota, phase, waveform_approximant, reference_frequency, ra, dec,
        geocent_time, psi):
 
    .... #The usual stuff using lalsim

    return {'plus': h_plus, 'cross': h_cross}


time_duration = 1.
sampling_frequency = 4096.
waveformgenerator = peyote.source.WaveformGenerator('BBH', sampling_frequency, time_duration, BBH)

# Inject a signal
waveformgenerator.set_parameters(mass_1=10, mass_2=5, ....) 
hf_signal = waveformgenerator.frequency_domain_strain(simulation_parameters)

# Simulate the data in H1
...
IFOs = [H1, L1]

likelihood = peyote.likelihood.likelihood(IFOs, waveformgenerator)

# Pass this likelihood and a prior to the sampler

Advantages

1. Can satisfy !13 (merged)
2. Easy for users to define their own models
3. Instance variables (i.e. mass_1 etc) not required at initialisation, but explicitly set before using (either for an injection or inference).

TO DO

• Add a default waveform generator (e.g. waveformgenerator = peyote.BBH)
• If accepted, should source.py be renamed?
• Figure out how to specify if it is a time-domain and frequency-domain and add the approach FFTing functions
• Update to work with !13 (merged)

Thoughts?

@paul-lasky @moritz.huebner @colm.talbot @rory-smith

I think this is a viable idea. The waveform is the main thing users are interested in at first anyway. In case somebody wants to make quick and dirty use of TUPAK just to generate a bunch of waveforms, this seems very intuitive. Given its importance, having a waveform generator is definitely a good idea.

The user would not have to think in which way the waveform generator is treating the variables internally. The user must just be aware of naming conventions for the parameters (which would be the case for any implementation, I think). The waveform generator could just generate any generic class and an instance based on this, which is handled internally.

The advantage on our end would be, that we can still use regular class structures and have a bunch of prefabricated and reusable functions that may go beyond frequency_domain_strain that can also be used with the waveform generator (In that case you would not have to give the waveform generator a function, the function would already be implemented in the source class).

added 78 commits

• a2706753...a5b62c57 - 76 commits from branch master
• 04f484b9 - Merge branch 'master' into update-how-sources-are-defined
• 95d453d3 - Working version of proposed changes with MR !13 (merged)

Compare with previous version

marked the checklist item Update to work with !13 (merged) as completed

marked the checklist item Add a default waveform generator (e.g. waveformgenerator = peyote.BBH) as completed

marked the checklist item If accepted, should source.py be renamed? as completed

added 1 commit

• 4b770a15 - Add docstrings and a general clean up

Compare with previous version

marked the checklist item Figure out how to specify if it is a time-domain and frequency-domain and add the approach FFTing functions as completed

added 16 commits

• 4b770a15...6ce06fab - 15 commits from branch master
• 10e93702 - Merge branch 'master' into update-how-sources-are-defined

Compare with previous version

Okay this is ready to merge.

The result of this MR is that the Source class and all its children are no longer used. Potentially this could be reverted by have the waveformgenerator have a source attribute. But I don't think it is necessary for now.

@moritz.huebner I'll assign it to you.

unmarked as a Work In Progress

Some specific changes that people might want to note

1. The prior variable passed to sampler should be a dictionary. In some previous commit, this was changed to being a source instance (and then subsequently called as prior.__dict__ in order to get round that it wasn't a dictionary). In future we may want to create out own Priors object and I've created an issue !16 (closed) to discuss this.

2. The Source objects are not longer passed around, but instead WaveformGenerator from the standpoint of much of the rest of the code these two are equivalent.

3. The waveformgenerator has an explicit parameter_keys list of parameters. This is intentional.

assigned to @moritz.huebner

Ok, I will merge this in tomorrow morning!

merged