Maybe we want to allow nonuniform time steps

[vijayvarma392]

There are some situations where non-uniform time steps are a very good idea:
Let's say we have a long waveform (BNS length). Then having a time step of 1M throughout would be very expensive. So, for example, the internal surrogate time array for NRHybSur3dq8 does non-uniform steps so that there are typically 5 points per orbit. This means very big steps in the early inspiral, very small steps near merger. We should think about how best to handle this situation.

Short fix could be revert to the use of np.gradient, which works fine for non-uniform steps.
Or, we can use the 4th order thing when the time steps are uniform, if not np.gradient.
Either way, we should limit, as much as possible, the number of places in the code where we rely on uniform steps.

[md-arif-shaikh]

Or maybe do interpolation on a uniform time array, if the input one is not uniform? I think this might be good, because, for example, in extrema finding code we might need to use a particular width which is the number of samples on either side to find the next peak or trough. If the time is non-uniform then this won't have a consistent meaning.

[vijayvarma392]

The problem with doing interpolation onto uniform time array is it's too memory intensive for such long waveforms; which is why nonuniform times are used in the first place. If you want dt small enough to resolve the peak (so we can set t=0 there), it will be a huge array for these cases. Ok, that width parameter is something to think about. Anything else in the code that needs uniform time steps?

[haraldp271]

Well, as far as I can see there is nothing in the documentation that states that only uniform time-steps are allowed. And I don't think the code tests for uniform time-steps. Hence, I think the code should be robust even if the time-steps are non-uniform.

I am aware of at least two parts where this comes up:

1. the width argument to find_peaks is a number of indices. Hence, its value has to be 'smart enough'. (see e.g. https://github.com/vijayvarma392/Eccentricity/blob/5bcd609fb217908bc36d6bf9a3ea6250a246de45/measureEccentricity/eccDefinitionUsingFrequencyFits.py#L344
2. When fitting the data-points near an extremum for a refined determination of the maximum, I saw somewhere a hardcoded number of 5 data-points. That should probably also be more variable, depending on dt and width of the maximum.

[vijayvarma392]

@md-arif-shaikh: Indeed the documentation should say that dataDict should have uniform time steps, can you add that? We can remove this later if this gets relaxed.

But, @haraldp271: there is already a check for this here.

Another thing that will need to change for nonuniform timesteps is how we compute omega: https://github.com/vijayvarma392/Eccentricity/blob/main/measureEccentricity/eccDefinition.py#L78. Right now, it uses a 4th-order accurate stencil that assumes uniform time steps. We could just switch to np.gradient if needed.

@md-arif-shaikh: Please document any other places where uniform timesteps are necessary in this issue.