neuroinformatics-unit / movement

Python tools for analysing body movements across space and time
http://movement.neuroinformatics.dev
BSD 3-Clause "New" or "Revised" License
96 stars 8 forks source link

Constrain kinematics functions to only accept Cartesian space #290

Closed niksirbi closed 1 month ago

niksirbi commented 1 month ago

Is your feature request related to a problem? Please describe. Spotted by @sfmig.

Working on the kinematics tests, I realise that right now we don't constrain our kinematic functions to take cartesian input only. The question is: should we? TLDR: I think yes As it is now, we can pass a position array in polar coordinates to any kinematics function, the only requirement is to have a "time" dimension. For example, if we do so for compute_displacement we would get a vector that holds the difference in rho (between consecutive frames) along the rho axis and and the difference in phi along the phi axis. Note that this is not the same as the displacement vector expressed in polar coordinates. I think this may be confusing to many people. It is easy to see this case if you imagine a particle moving in circular motion around the origin. The position vectors between consecutive frames will have constant rho in polar coordinates, so the computed "displacement vector" will have rho=0. But the actual displacement vector (the vector from the current to the previous position) does have some length, and therefore a rho!=0. A similar situation applies to compute_velocity and compute_acceleration, but it is easier to see for the velocity. If we pass a position array in polar coordinates to compute_velocity, the resulting velocity vector will have the rate of change in rho along the rho axis, and the rate of change in phi along the phi axis. This is not the same as the components of the velocity vector in polar coordinates.

Describe the solution you'd like

Restrict the kinematic functions to cartesian only -- I'd say this is preferred

Describe alternatives you've considered

Add implementations to compute kinematics of a position array in polar coordinates -- I don't think this would be a good use of our time, since in practice we would just do the transform under the hood anyways

Additional context

If anything, we could consider adding functionality to compute radial and tangential velocity / acceleration, which are much more used.

See issue #289.

niksirbi commented 1 month ago

Closing this as duplicate. See #291