Save correct luminosity distance when distance marginalizing

Currently, when we marginalize over distance only the reference distance of 10 Mpc is saved into the result file. Working with result files than might require you to manually dig through the log to figure out the true luminosity distance.

changed milestone to %1.0.0

added Bug Feature-request Good First Issue + 1 deleted label

If I understood the discussion correctly, #323 (closed) is addressing this for time-marginalization.

However, I'm a bit confused by "digging through the results files to figure out the true luminosity distance". What is meant by the true luminosity?

My understanding is that we currently just save the lower bound of the prior. When we should't save anything (since we didn't sample the prior). Then the full fix is to resample and add those to the posterior after the fact. Am I missing something?

Recovering the distance posterior has been possible since !249 (merged), !317 (merged) will extend this to time and phase.

By "true" value do you mean the injected value? That's stored independently of the posterior.

Can we close this? We have loads of open issues on very similar topics, e.g., #324 (closed) #323 (closed).

It is related to this bit of code in GravitationalWaveTransient:

        if self.distance_marginalization:
            self._check_prior_is_set(key='luminosity_distance')
            self._distance_array = np.linspace(self.priors['luminosity_distance'].minimum,
                                               self.priors['luminosity_distance'].maximum, int(1e4))
            self._setup_distance_marginalization()
            priors['luminosity_distance'] = float(self._ref_dist)

This changes the prior that was passed in inadvertently. I think it may end up being confusing and causing unintended behaviour. I just spent about 2 hours tracking down unexpected behaviour that was related to this. I had a loop that was creating GravitationalWaveTransient objects with distance_marginalization=True, however, the prior was defined outside the loop and was hence overwritten after the first iteration.

I also had a problem in which the saved injected value for the luminosity distance in the result object ended up being 10 (i.e. the reference distance for distance marginalization). This may be related to how I wrote this.

To be honest, I don't know any better way of implementing this.

This isn't "inadvertent", we discussed it at some length and decided this was the behaviour we wanted. These might benefit from being better documented to spell this out and how to recover the posteriors.

This is solved right? Provided you pass the generate_all function correctly it will be correctly reconstructed.

I"m going to close this and open a new issue #336 (closed)

closed