BUG: Fix GWSignalWaveformGenerator reconstruction in waveform posterior plots

[lorenzopompili00]

I was trying to plot some waveform reconstructions using plot_interferometer_waveform_posterior for some eccentric PE runs using the new GWSignalWaveformGenerator, and I realized it is currently not working correctly.

The reason is that the arguments required to instantiate GWSignalWaveformGenerator are not saved in the likelihood metadata, and are not passed to the waveform generator when it is instantiated in plot_interferometer_waveform_posterior here.

A fix is to save and pass these extra arguments whenever the waveform generator is GWSignalWaveformGenerator. I have added this via a waveform_generator_constructor_dict, inspired by this bilby_pipe merge request. Let me know if you have any other preferences for handling this.

[ColmTalbot]

@lorenzopompili00 thanks for opening this PR! I think we should add this functionality, but I'd like to think about whether we can find a more generic way of implementing this.

To be concrete, I'd suggest adding a new property to the WaveformGenerator like

class WaveformGenerator:
    ...
    @property
    def meta_data(self):
        return dict()


class GWSignalWaveformGenerator:
    ...
    @property
    def meta_data(self):
        return dict(
            eccentricity=...
        )

We could also put in the information needed to reconstruct the WaveformGenerator for that class.

I'm interested to get thoughts from a wider group on this.

[ColmTalbot]

Maybe we should just add a generic meta_data attribute to the waveform generator as for interferometers and then add that here? That would avoid having a special case here.

[mj-will]

@lorenzopompili00 thanks for opening this PR! I think we should add this functionality, but I'd like to think about whether we can find a more generic way of implementing this.

To be concrete, I'd suggest adding a new property to the WaveformGenerator like

class WaveformGenerator:
    ...
    @property
    def meta_data(self):
        return dict()


class GWSignalWaveformGenerator:
    ...
    @property
    def meta_data(self):
        return dict(
            eccentricity=...
        )

We could also put in the information needed to reconstruct the WaveformGenerator for that class.

I'm interested to get thoughts from a wider group on this.

I like this suggestion, and would be in favour of adding meta_data to the generators.

[lorenzopompili00]

Thanks for taking a look and for the feedback @ColmTalbot @mj-will! I agree your suggestion is cleaner, I have made changes to address it.

[ColmTalbot]

Thanks for making the changes, I'm happy with this version.