Add Pose2 Component Jacobians #1504
Conversation
Adds jacobians to translation() and rotation() for Pose2 to bring it into spec with Pose3's equilivent functions. Also adds tests.
gtsam/geometry/Pose2.h
Outdated
| @@ -252,16 +252,16 @@ class Pose2: public LieGroup<Pose2, 3> { | |||
| inline double theta() const { return r_.theta(); } | |||
|
|
|||
| /// translation | |||
| inline const Point2& t() const { return t_; } | |||
| inline const Point2& t() const { return translation(); } | |||
There was a problem hiding this comment.
translation() should also be inline otherwise this makes no sense?
There was a problem hiding this comment.
Agreed, at the moment t() and r() are effectively macros to translation and rotation respectively. Regarding the design decision I can see an argument on each side of making these inline. I am happy to update to the preference of the maintainers!
The parallel Pose3 functions are note inline if that provides signal to the decision process.
There was a problem hiding this comment.
Seems that this orthogonal to the aim of this PR. What's the problem with leaving these methods as is?
There was a problem hiding this comment.
No problem, just my own over-dedication to the one source of truth principle. Reverted!
gtsam/geometry/Pose2.h
Outdated
| @@ -252,16 +252,16 @@ class Pose2: public LieGroup<Pose2, 3> { | |||
| inline double theta() const { return r_.theta(); } | |||
|
|
|||
| /// translation | |||
| inline const Point2& t() const { return t_; } | |||
| inline const Point2& t() const { return translation(); } | |||
There was a problem hiding this comment.
Seems that this orthogonal to the aim of this PR. What's the problem with leaving these methods as is?
|
Merge in develop to re-run windows CI? |
Motivation
Pose3 component extraction functions (
rotationandtranslation) optionally allow for jacobians. However, Pose2 does not parallel this interface. This can cause issues when defining code templated to handle either dimension of pose.Implementation
This PR updates the Pose2 interface to parallel the Pose3 interface by adding
OptionalJacobiansto Pose2'stranslationandrotationfunctions.The derivation of these Jacobians directly parallels that for Pose3, although we do have to account for the difference in tangent space parameter order between Pose2 and Pose3.
Jacobians are verified with comparison to numerical Jacobian in added tests.
The python interface is also extended to support these changes.
Effects
While these are interface level changes, they should not cause any backwards compatibility issues, as they are implemented with
OptionalJacobiansthat default tonone. Existing code should be agnostic to these changes.