start_time missing from times_lower/times_upper in linear interpolation

[binary69]

In SequenceInterpolator.interpolate and PhaseShiftedSequenceInterpolator.interpolate, times_lower and times_upper are computed without adding start_time

idx_lower is derived by idx_lower = np.floor((valid_times - self.start_time) / self.time_delta).astype(int )
but times_lower , is calculated as times_lower = idx_lower * self.time_delta

for start_time != 0 it'll be innacurate in the numerator

lower_signal_ratio = ((times_upper - times_valid) / denom)[:, None] 
upper_signal_ratio = ((times_valid - times_lower) / denom)[:, None]

fix would be:

times_lower = self.start_time + idx_lower * self.time_delta 
times_upper = self.start_time + idx_upper * self.time_delta

Happy to open a PR if this is something that's to be addressed!

[pollytur]

Thanks for a catch @binary69
Before submitting a pr - could you please make a toy example / test case, showing that its actually a problem
I will look in the code more detailed in the meanwhile

[binary69]

In the existing tests they are taking start_time = 0

but for example:

start_time =  5
time_delta = 0.1
valid_time= 5.04

idx_lower  = (5.04 - 5)/0.1      #0 
idx_upper = 0+1    #1 

#current code    
times_lower = 0
times_upper = 0.1
denom = 0.1 

lower_signal_ratio = ((0.1 - 5.04) / 0.1)   #-49.4
upper_signal_ratio = ((5.04 -0 ) / 0.1)  #50.4

#fix 
times_lower = 0+5 #5
times_upper = 5 + 0.1  #5.1 
denom = 0.1 #unchanged

lower_signal_ratio = ((5.1 - 5.04) / 0.1)   #0.6
upper_signal_ratio = ((5.04 -5 ) / 0.1)  #0.4

[pollytur]

@binary69 alright, thanks a lot, looks like a bug for me
Please make a PR with the fix and additional tests and ping me and @schewskone to review

@schewskone if you could take a look at the issue and confirm its a problem - would be great as well :)