neuroinformatics-unit/movement

Remove equidistant time-spacing assumption in computing approximate derivative

lochhh opened this issue · 6 comments

Currently when computing approximate derivatives, we assume equidistant time-spacing with a fixed dt:

dt = data["time"].values[1] - data["time"].values[0]
for _ in range(order):
result = xr.apply_ufunc(
np.gradient,
result,
dt,
kwargs={"axis": 0},
)
result = result.reindex_like(data)

We should instead provide the actual time axis values, i.e. data.coords["time"].values.
Since we do not need to compute derivatives along multiple axes (what np.gradient() can do and xr.differentiate() can't), an even simpler solution would be to use xr.differentiate() (as @sfmig has pointed out in #265), replacing the above lines with:

for _ in range(order): 
     result = result.differentiate("time")

I wonder if they differ in performance - maybe we could run a quick time test comparing our current implementation and differentiate 🤔

I wonder if they differ in performance - maybe we could run a quick time test comparing our current implementation and differentiate 🤔

I think they use np.gradient().

This sounds like an unambiguously good idea to me 👍🏼

I'm also all in for using xr.differentiate().

I wonder if they differ in performance - maybe we could run a quick time test comparing our current implementation and differentiate 🤔

# input:
dlc_ds.postion.shape = (59999, 2, 12, 2)

# xr.differentiate
%%timeit
dlc_ds.position.differentiate("time")
# 24.2 ms ± 1.56 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

# numpy
result = xr.apply_ufunc(
    np.gradient,
    dlc_ds.position,
    dlc_ds.time.values,
    kwargs={"axis": 0},
)
# 23.4 ms ± 652 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

but because we need to reindex after applying np.gradient, this takes longer

%%timeit
result = xr.apply_ufunc(
    np.gradient,
    dlc_ds.position,
    dlc_ds.time.values,
    kwargs={"axis": 0},
)
result = result.reindex_like(dlc_ds.position)
# 32 ms ± 1.51 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

@lochhh thanks for timing it 🤩