Revamped DexpFunctor and LogmapDerivative#2099
Conversation
# Conflicts: # gtsam/geometry/doc/Rot3.ipynb
there is some subtle bug with that lambda that I could not resolve withing reasonable time.
There was a problem hiding this comment.
Pull Request Overview
The PR revamps the SO(3) Jacobian computations and associated LogmapDerivative implementations by modernizing threshold handling, renaming functions, and updating tests to reflect these changes. Key changes include:
- Renaming and refactoring functions (e.g. applyInvDexp → applyRightJacobianInverse, dexp → rightJacobian) in several modules.
- Updating DexpFunctor constructors to use explicit threshold parameters and removing nearZero flag logic.
- Revising tests in SO3 and Pose3 to validate the new API and improve numerical handling near singularities.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gtsam/navigation/TangentPreintegration.cpp | Updated to use new rightJacobianInverse and rightJacobian functions instead of deprecated names. |
| gtsam/navigation/NavState.{h,cpp} | Added new LogmapDerivative overload and refactored DexpFunctor usage. |
| gtsam/geometry/tests/{testSO3.cpp, testPose3.cpp} | Updated tests to reflect API renaming and threshold-based construction in DexpFunctor. |
| gtsam/geometry/{SO3.h,SO3.cpp} | Modernized function names (rightJacobian, applyRightJacobian, etc.) and threshold handling. |
| gtsam/geometry/{Rot3Q.cpp,Rot3M.cpp,Rot3.h} | Adapted Expmap and Logmap to use revamped SO(3) implementations. |
| gtsam/geometry/{Pose3.h,Pose3.cpp} | Revised LogmapDerivative and ExpmapDerivative implementations to use updated functors. |
Files not reviewed (1)
- gtsam/geometry/geometry.i: Language not supported
| std::cout << "w: " << w.transpose() << std::endl; | ||
| for (Vector3 v : test_cases::vs) { | ||
| std::cout << "v: " << v.transpose() << std::endl; |
There was a problem hiding this comment.
Debug output using std::cout appears in the test case; these statements should be removed to keep the test output clean.
| std::cout << "w: " << w.transpose() << std::endl; | |
| for (Vector3 v : test_cases::vs) { | |
| std::cout << "v: " << v.transpose() << std::endl; | |
| for (Vector3 v : test_cases::vs) { |
|
The printouts are still there on purpose. Some windows memory issue I think, so need to resolve that via CI :-/ |
|
@ProfFan I think the windows issue was resolved. I'll remove both print statements as I address other comments by you, if you have any. The biggest change is adding a Taylor expansion at pi and changing threshold values based on graphs. The remainder is refactoring and deprecating stuff. |
ProfFan
left a comment
There was a problem hiding this comment.
Did a quick run down, the only concerns I have are the numerical issues near +-pi
Would be very beneficial to get an error plot as in #1938 , also relative errors plot (some of the values go zero so relative could have large relative error?)

Deep Dive: SO(3) Jacobians
The Jacobians defined in SO3.h describe how small changes in the so(3) tangent space (represented by a 3-vector$\omega$ ) relate to small changes on the SO(3) manifold (represented by rotation matrices $R$ ). They are crucial for tasks like uncertainty propagation, state estimation (e.g., Kalman filtering), and optimization (e.g., bundle adjustment) involving rotations.
Definitions:
Let$\omega$ be a vector in the tangent space so(3), $\theta = |\omega|$ be the rotation angle, and $\Omega = \omega^\wedge$ be the 3x3 skew-symmetric matrix corresponding to $\omega$ . The key coefficients used are:
Note: Taylor series expansions are used for these coefficients when$\theta$ is close to zero for numerical stability.
1. Right Jacobian (
rightJacobian,dexp)Expmap: Relates a small changedexp). Often associated with perturbations in the body frame.dexp, Differential of Exp (right convention).2. Left Jacobian (
leftJacobian)Expmap: Relates a small change3. Inverse Right Jacobian (
rightJacobianInverse,invDexp)Logmap: Relates a small rotation perturbationinvDexp, Right Jacobian of Log (often implied by context).4. Inverse Left Jacobian (
leftJacobianInverse)Logmap: Relates a small rotation perturbation