Conversation
There was a problem hiding this comment.
Pull request overview
This PR homogenizes the Kalman filter implementations by introducing a shared stateful BaseKalmanFilter API, refactoring the AABB (xysr) and OBB (xywha) Kalman filters to use it, and adding focused unit tests for OBB-specific angle/affine behavior.
Changes:
- Expanded
BaseKalmanFilterto own stateful KF matrices/state and provide shared predict/update/project helpers. - Refactored
KalmanFilterXYSRandKalmanFilterXYWHAto inherit fromBaseKalmanFilter, adding OBB measurement normalization (angle wrapping/shortest residual) and improved affine correction handling. - Added unit tests covering XYWHA inheritance, unfreeze behavior after missing observations, angular wrap residuals, and affine correction effects.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/test_xywha_kf.py | Adds regression/unit tests for XYWHA unfreeze, angular residual wrapping, and affine correction behavior. |
| boxmot/motion/kalman_filters/obb/xywha_kf.py | Refactors XYWHA KF to inherit from BaseKalmanFilter, adds angle wrapping + measurement prep, improves affine correction and unfreeze angle handling. |
| boxmot/motion/kalman_filters/aabb/xysr_kf.py | Refactors XYSR KF to inherit from BaseKalmanFilter and route predict/update through shared base helpers while preserving freeze/unfreeze behavior. |
| boxmot/motion/kalman_filters/aabb/base_kalman_filter.py | Introduces stateful KF members and shared matrix-based predict/update/project/MD helpers to unify KF implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| self.x = self.x + np.dot(self.K, self.y) | ||
| self.P = self.P - np.linalg.multi_dot((self.K, projected_cov, self.K.T)) |
There was a problem hiding this comment.
update_state() updates covariance with P = P - K S K^T. Elsewhere in this codebase (e.g., KalmanFilterXYWHA.update() and HybridSort’s kalman filter) the Joseph form is used explicitly for numerical stability/PSD preservation. Since update_state() is now used by KalmanFilterXYSR.update(), consider switching update_state() to the Joseph form (or at least symmetrizing and enforcing PSD) to reduce the risk of P becoming non-symmetric/indefinite and breaking later Cholesky-based operations (e.g., Mahalanobis distance).
| self.P = self.P - np.linalg.multi_dot((self.K, projected_cov, self.K.T)) | |
| # Joseph form covariance update for numerical stability: | |
| # P = (I - K H) P (I - K H)^T + K R K^T | |
| I_KH = np.eye(self.dim_x) - self.K @ H | |
| self.P = I_KH @ self.P @ I_KH.T + self.K @ R @ self.K.T | |
| # Enforce symmetry | |
| self.P = 0.5 * (self.P + self.P.T) | |
| # Enforce positive semi-definiteness via eigenvalue clipping | |
| eigvals, eigvecs = np.linalg.eigh(self.P) | |
| eigvals_clipped = np.clip(eigvals, 0.0, None) | |
| self.P = (eigvecs * eigvals_clipped) @ eigvecs.T |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| delta_t=self.delta_t, | ||
| Q_xy_scaling=self.Q_xy_scaling, | ||
| Q_s_scaling=self.Q_s_scaling, | ||
| Q_a_scaling=self.Q_s_scaling, |
There was a problem hiding this comment.
When creating new trackers for unmatched detections, Q_a_scaling is being set to self.Q_s_scaling. This makes the angle/rotation process noise scaling identical to the scale noise (and ignores any intended separate tuning for OBB angle dynamics). Either pass a dedicated self.Q_a_scaling value (add it to OcSort.__init__ and store it), or remove Q_a_scaling from the call if it is not meant to be configurable here.
| Q_a_scaling=self.Q_s_scaling, |
No description provided.