feat: implement compute_path_sinuosity core logic (draft)#981
Conversation
|
| from movement.kinematics.kinematics import ( | ||
| compute_backward_displacement, | ||
| compute_forward_displacement, | ||
| compute_turning_angle, |
There was a problem hiding this comment.
compute_turning_angle is in movement.kinematics.orientation not in movement.kinematics.kinematics. You can either import from the correct specific submodule, or else import all 3 functions from the higher-level module movement.kinematics (both should work I think.)
| # Reuse the internal validation helper (checks for >= 2 points) | ||
| data = _slice_and_validate(data, start, stop, "path sinuosity") | ||
|
|
||
| # Sinuosity specifically needs 3 points for angles | ||
| n_time = data.sizes["time"] | ||
| if n_time < 3: | ||
| raise logger.error( | ||
| ValueError( | ||
| "At least 3 time points are required to compute sinuosity " | ||
| f"(got {n_time}). A minimum of two consecutive displacements " | ||
| "is needed to produce one turning angle." | ||
| ) | ||
| ) |
There was a problem hiding this comment.
I wonder if this is a good chance to modify _slice_and_validate so it accepts min_points kwarg (defaulting to 2). Then we could fold all this validation into one helper.
| nan_policy: Literal["ffill", "scale"] = "ffill", | ||
| nan_warn_threshold: float = 0.2, | ||
| ) -> xr.DataArray: | ||
| r"""Compute the sinuosity of a path (Benhamou 2004). |
There was a problem hiding this comment.
Re the docsting, I would add the following sections:
- See Also: (linking to underlying function
compute_turning_angle, and to the existing related metriccompute_path_straightness) - Notes: containing the reference and the math equations, and also a short note about sinuosity having units of 1/sqrt(length), and therefore interpretation depends on position units.
- Examples: similar to the other functions in this file (tackle only after the function signature is settled)
| ) | ||
|
|
||
| # --- Step lengths ------------------------------------------------------- | ||
| step_lengths = compute_norm(compute_forward_displacement(data)) |
There was a problem hiding this comment.
after this line I would add:
step_lengths = step_lengths.where(step_lengths > min_step_length)The current implementation applies min_step_length assymetrically. Small steps are masked in turning_angles (so exlcuded from step_lengths (kept it
Masking them in both makes sense, because the intention of min_step_length in compute_turning_angle is to exlude steps we consider as 'noise'.
| if nan_policy == "ffill": | ||
| data = data.ffill(dim="time") | ||
| elif nan_policy != "scale": | ||
| raise logger.error( | ||
| ValueError( | ||
| f"Invalid value for nan_policy: {nan_policy}. " | ||
| "Must be one of 'ffill' or 'scale'." | ||
| ) | ||
| ) |
There was a problem hiding this comment.
I don't think that these NaN policies (originally made for path length) make much sense here.
I would remove the nan_policy kwarg entitely, and just let NaNs propagate, but compute all the statistics with skipna=True. This effectively keeps the current behaviour of the 'scale' branch, but it's not worth calling it a 'policy' here.
I would keep nan_warn_threshold as a still useful sanity check. Optionally you can add a paragraph to 'Notes' like this:
NaN positions propagate to NaN step lengths and turning angles; the statistics are computed over the remaining valid samples.
With the above in mind, the argument order in the function signature should be:
data, start, stop, nan_warn_threshold, min_step_length
| result = 2.0 * (p_bar_safe * (angle_term + b**2)) ** -0.5 | ||
|
|
||
| result = xr.where(is_straight, 0.0, result) | ||
| result = xr.where(is_stationary, np.nan, result) |
There was a problem hiding this comment.
I think this final xr.where(is_stationary, np.nan, result) is redundant. p_bar_safe is already NaN where stationary, so it should propagate to the result anyway.
2 participants
Opening this as a draft to get early eyes on the core logic and math implementation (Benhamou 2004).
Key implementation details:
I will start adding the pytest suite for this once the core mathematical approach looks good.