Implement geophysical measurement Jacobians for EKF updates#228
Merged
Conversation
Co-authored-by: jbrodovsky <57160841+jbrodovsky@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add geophysical measurement model Jacobians for EKF updates
Implement geophysical measurement Jacobians for EKF updates
Jan 15, 2026
Owner
|
@copilot Formatting checks are failing in the CI battery. Please ensure that your pull request is properly formatted and linted. |
Co-authored-by: jbrodovsky <57160841+jbrodovsky@users.noreply.github.com>
Contributor
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements measurement Jacobians for geophysical measurements (gravity and magnetic anomalies) to enable correct EKF updates. Previously, these measurements incorrectly fell back to using GPS position Jacobians. The solution adds an optional get_jacobian method to the MeasurementModel trait and implements it for both geophysical measurement types.
Changes:
- Added optional
get_jacobian()method toMeasurementModeltrait with default implementation returningNone - Modified EKF update logic to check for measurement-provided Jacobians before falling back to type-based dispatch
- Implemented
get_jacobian()forGravityMeasurementandMagneticAnomalyMeasurementusing numerical gradients from geophysical maps - Added comprehensive unit tests for the trait-based Jacobian functionality
- Updated documentation in linearize.rs to reference actual implementations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/measurements.rs | Adds optional get_jacobian method to MeasurementModel trait with comprehensive documentation |
| core/src/kalman.rs | Updates EKF update function to prioritize measurement-provided Jacobians over type dispatch |
| geonav/src/lib.rs | Implements get_jacobian for geophysical measurements and adds tests verifying trait method works correctly |
| core/src/linearize.rs | Updates documentation for placeholder functions to reference actual implementations in geonav crate |
Comments suppressed due to low confidence (2)
geonav/src/lib.rs:614
- The conversion from per-degree to per-radian is incorrect. The
get_gradientfunction returns gradients in units of (map_value change)/(degree change). To convert to (map_value change)/(radian change), you need to multiply byDEG_TO_RAD(approximately 57.3), notRAD_TO_DEG(approximately 0.0175). The current code is dividing by approximately 57.3 when it should be multiplying. This will cause the Jacobian values to be off by a factor of ~3283. The correct conversion should be:h[(0, 0)] = dlat_deg / RAD_TO_DEG;or equivalentlyh[(0, 0)] = dlat_deg * DEG_TO_RAD;whereDEG_TO_RAD = PI / 180.0.
h[(0, 0)] = dlat_deg * RAD_TO_DEG;
h[(0, 1)] = dlon_deg * RAD_TO_DEG;
geonav/src/lib.rs:718
- The conversion from per-degree to per-radian is incorrect. The
get_gradientfunction returns gradients in units of (map_value change)/(degree change). To convert to (map_value change)/(radian change), you need to multiply byDEG_TO_RAD(approximately 57.3), notRAD_TO_DEG(approximately 0.0175). The current code is dividing by approximately 57.3 when it should be multiplying. This will cause the Jacobian values to be off by a factor of ~3283. The correct conversion should be:h[(0, 0)] = dlat_deg / RAD_TO_DEG;or equivalentlyh[(0, 0)] = dlat_deg * DEG_TO_RAD;whereDEG_TO_RAD = PI / 180.0.
h[(0, 0)] = dlat_deg * RAD_TO_DEG;
h[(0, 1)] = dlon_deg * RAD_TO_DEG;
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.
Implementation Complete: Geophysical Measurement Jacobians for EKF
✅ All Tasks Completed
get_jacobianmethod toMeasurementModeltraitupdatefunction to check for measurement-provided Jacobiansget_jacobianfor geophysical measurements in geonav crateRecent Changes
Fixed formatting (commit: latest)
measurements.rsline 109Solution Summary
Changes Made:
Added
get_jacobian()toMeasurementModeltrait (core/src/measurements.rs)Option<DMatrix<f64>>Nonefor backward compatibilityUpdated EKF update function (core/src/kalman.rs, line 888-926)
measurement.get_jacobian()FIRST before type-based dispatchImplemented Jacobians for geophysical measurements (geonav/src/lib.rs)
GravityMeasurement::get_jacobian()returns Jacobian via traitMagneticAnomalyMeasurement::get_jacobian()returns Jacobian via traitAdded comprehensive tests (geonav/src/lib.rs)
test_gravity_measurement_trait_jacobiantest_magnetic_measurement_trait_jacobianUpdated documentation
Test Results
Technical Implementation
Geophysical measurements compute Jacobians using numerical gradients from map data:
This matches the mathematical formulation from the issue comment and properly computes the measurement model linearization for EKF updates.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.