fix ordering after interpolation

There's a bug when using both time and distance marginalisation with a reduced time window for the prior as noticed in !296 (merged).

The interpolation in the distance lookup was sorting the output likelihoods. This MR fixes that by rearranging the output likelihoods to have the original indices. See https://stackoverflow.com/questions/44941271/scipy-interp2d-returned-function-sorts-input-argument-automatically-and-undesira for where the solution came from.

Another option would be to define a new class, e.g.,

from scipy.interpolate import interp2d
import numpy as np

class unsorted_interp2d(interp2d):
    def __call__(self, x, y, dx=0, dy=0):
        unsorted_idxs = np.argsort(np.argsort(x))
        return super().__call__(x, y, dx=dx, dy=dy)[unsorted_idxs]

and replace interp2d with unsorted_interp2d in line 253.

I have to say, Matt's solution looks slightly more elegant and avoid carrying around an "unsorted_idxs" array. I'd be happy with either solution, but for either case I think there needs to be some comment/documentation on why this is here. Even simply a link to this MR?

added 1 commit

• a97b03cf - add unsorted interpolation class

Compare with previous version

Yeah, that looks great, I just pushed a version.

A related issue is that the distance marginalised likelihood doesn't run on the CI as it takes a few minutes to build. We should think about how to add a few tests which use the distance marginalisation, without taking too long to run.

added 1 commit

• e4e671f8 - remove useless code

Compare with previous version

approved this merge request

added 1 commit

• fb7c1e21 - fix call to super

Compare with previous version

approved this merge request

approved this merge request

merged

mentioned in commit 8679c5c4

mentioned in merge request !296 (merged)