-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove equidistant time-spacing assumption in computing approximate derivative #268
Comments
I wonder if they differ in performance - maybe we could run a quick time test comparing our current implementation and |
I think they use |
This sounds like an unambiguously good idea to me 👍🏼 |
I'm also all in for using |
# 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 🤩 |
Currently when computing approximate derivatives, we assume equidistant time-spacing with a fixed
dt
:movement/movement/analysis/kinematics.py
Lines 105 to 113 in e7ccfe5
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 andxr.differentiate()
can't), an even simpler solution would be to usexr.differentiate()
(as @sfmig has pointed out in #265), replacing the above lines with:The text was updated successfully, but these errors were encountered: