fix(motion): validate homography and renormalize the accumulator - #560
Draft
AlexBodner wants to merge 1 commit into
Draft
fix(motion): validate homography and renormalize the accumulator#560AlexBodner wants to merge 1 commit into
AlexBodner wants to merge 1 commit into
Conversation
Two defects that compound into a permanently broken world frame: `HomographyTransformation.__init__` inverted the matrix with no guard, so a singular or non-finite matrix surfaced a bare `LinAlgError` from deep inside numpy. It now raises `ValueError` like the existing shape check, so the class has one documented failure mode. `MotionEstimator._estimate_homography` accumulated `H_current @ H_total` without renormalizing, so the projective scale compounded every frame and drifted toward a degenerate matrix. It also assigned the accumulator before constructing the transformation, so once a bad matrix landed there it was never cleared and every later frame inherited it. The scale is now pinned back to 1 after each accumulation, and the accumulator is only committed once the matrix proves invertible; if it does not, the world frame is re-baselined with a warning instead of raising mid-sequence. Adds tests/motion/test_transformation.py, which had no coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves homography validation and prevents invalid motion accumulation from permanently corrupting the world frame.
Changes:
- Validates finite and invertible homography matrices.
- Renormalizes accumulated homographies and re-baselines invalid state.
- Adds transformation and estimator regression tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/trackers/motion/transformation.py |
Adds homography validation. |
src/trackers/motion/estimator.py |
Normalizes and safely commits accumulated motion. |
tests/motion/test_transformation.py |
Tests transformation behavior and validation. |
tests/motion/test_estimator.py |
Tests normalization and recovery. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+253
to
+255
| scale = accumulated_homography[2, 2] | ||
| if np.isfinite(scale) and abs(scale) >= _MIN_HOMOGRAPHY_SCALE: | ||
| accumulated_homography = accumulated_homography / scale |
Comment on lines
+108
to
+113
| try: | ||
| self.inverse_homography_matrix = np.linalg.inv(self.homography_matrix) | ||
| except np.linalg.LinAlgError as error: | ||
| raise ValueError( | ||
| f"Homography matrix is singular and cannot be inverted, got:\n{self.homography_matrix}" | ||
| ) from error |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two defects that compound into a permanently broken world frame.
HomographyTransformation.__init__inverts the matrix with no guard, so a singular or non-finite matrix surfaces a bareLinAlgErrorfrom inside numpy, even though the class already validates shape with a clearValueError.MotionEstimator._estimate_homographyaccumulatesH_current @ H_totalwithout renormalizing. The projective scale compounds every frame and drifts toward a degenerate matrix. Worse, the accumulator is assigned before the transformation is constructed, so once a bad matrix lands there it is never cleared and every later frame inherits it.Fix
HomographyTransformationraisesValueErrorfor non-finite and singular matrices, so the class has one documented failure mode.Tests
Adds
tests/motion/test_transformation.py— the module had no coverage at all.ValueError.LinAlgErrorbefore the change).tests/motionandtests/utilspass at 288.Notes
The estimator swallows the
ValueErrorand re-baselines rather than propagating, because tracking should survive a bad frame. The public constructor still raises, which is what a direct caller wants.Made with Cursor