Repository navigation
solve errors in acceleration computation and adjust user guide - #588
JuleAdrian wants to merge 7 commits into
Conversation
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
schroedtert
left a comment
schroedtert
left a comment
There was a problem hiding this comment.
1. Design question: stencil width for frame_step
I checked the math with x = t², y = −3t², and the results match the analytic acceleration for frame_step 1 and 2. The 0.5 factor in time_interval is correct: window_size spans 4n frames, but the sample spacing is 2n.
My question is about the semantics of frame_step. With frame_step = n:
- speed uses X(t±n), so the stencil reaches ±n frames and Δt = 2n/fps
- acceleration now uses X(t±2n), so the stencil reaches ±2n frames, with h = 2n/fps
So the sameframe_stepsmooths acceleration over twice the window it uses for speed. The docstring does say Δt = 2n/fps, but it doesn't say the stencil is wider than for speed. Two consequences:
Users who pass the same frame_step to speed and acceleration probably expect the same time window.
2. Missing changelog entry
3. Missing unit tests (adding them later in a different PR would be okay for me, but having them directly would be better)
| bidirectional (bool): if True also the future frame_step points | ||
| will be used to determine the movement |
There was a problem hiding this comment.
Can you remove this argument as it not used (even though you didn't introduce the change here)?
| df_movement[START_POSITION_COL] = df_movement.groupby(by=ID_COL).point.shift(-2 * frame_step) | ||
| df_movement["start_frame"] = df_movement.groupby(by=ID_COL).frame.shift(-2 * frame_step) |
There was a problem hiding this comment.
start and end are swapped. Start uses shift(-2n), which is the future, and end uses shift(2n), which is the past. Purely semantics, otherwise here and the speed movement calculation use different notations for start and end
| df_movement["end_frame"] = df_movement.groupby(by=ID_COL).frame.shift(2 * frame_step) | ||
|
|
||
| df_movement[WINDOW_SIZE_COL] = df_movement.end_frame - df_movement.mid_frame | ||
| df_movement[WINDOW_SIZE_COL] = df_movement.end_frame - df_movement.start_frame |
There was a problem hiding this comment.
this is where window_size becomes −4n.
| if movement_direction is not None: | ||
| # Projection of the displacement onto the movement direction | ||
| norm_movement_direction = np.dot(movement_direction, movement_direction) | ||
| movement_data[["dd_x", "dd_y"]] = ( | ||
| np.dot(movement_data[["dd_x", "dd_y"]].to_numpy(), movement_direction)[:, None] | ||
| * movement_direction | ||
| * norm_movement_direction | ||
| ) | ||
| movement_data[ACC_COL] = ( | ||
| np.dot(movement_data[["dd_x", "dd_y"]].to_numpy(), movement_direction) | ||
| / np.linalg.norm(movement_direction) | ||
| / time_interval**2 | ||
| / (time_interval**2) | ||
| ) |
There was a problem hiding this comment.
The projection multiplies by |d|². d should be normalised, which never happens. If you want you can fix it in this PR or open a bug-ticket for it.
| frame_step (int): how many frames back and forwards are used to compute | ||
| the movement. |
There was a problem hiding this comment.
No longer valid as now 2*frame_step is taken for computation
closes #548
Made some corrections in the calculation of the acceleration. The final formula is
a_i(t_k) = ( X(t_{k+2n}) - 2X(t_k) + X(t_{k-2n}) ) / \delta t^2with
\delta t = 2n/fps.I also adjusted the user guide to make it easier to understand.