feat(math): add SE2(3) extended-pose Lie group — the R-IEKF state manifold (#347) - #353
Conversation
…ifold (#347) Phase B foundation. SE2(3) (R, v, p) is the extended-pose / double-direct-isometry group: a rotation plus two coupled translations (velocity, position). Writing the IMU state here makes the dynamics group-affine, so the right-invariant error Jacobians are state-independent and the unobservable yaw/position subspace is preserved by construction — the cure Phase A validated (flat yaw leak, state-independent propagation Phi). - math/lie/se23.hpp: exp/log (both translations share the SO3 left Jacobian), compose, inverse, 5x5 matrix, 9x9 adjoint, in the rotation-first [phi; nu; rho] ordering. Mirrors the se3.hpp idioms; clean-room (Sola 2018, Barrau-Bonnabel 2017). - lie_groups tests: exp/log round-trip, compose/inverse, and the adjoint identity X.exp(xi).X^-1 = exp(Ad_X.xi). Runs alongside the existing SO3/SE3/Sim3 suite. Next: the invariant IMU propagation and camera measurement update on this state. Refs #347, #348, #212. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR introduces the SE₂(3) "extended pose" Lie group as a new C++20 header-only template class with rotation, velocity, and position components. It implements exp/log (using SO(3) Jacobians), group composition and inversion, 5×5 and 9×9 matrix/adjoint representations, and adds tests validating round-trips, group identities, adjoint action, multi-arithmetic coverage, and a near-half-turn regression. ChangesSE₂(3) Extended Pose Lie Group Implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/math/lie_groups.cpp`:
- Around line 412-450: SE23 tests only exercise double precision; add
multi-arithmetic round-trip and near-singular tests to match SO3/SE3/Sim3. Add a
TEST_CASE "SE23 round-trip across arithmetic types" that recreates the double
round-trip using lie::SE23<float> with det::Vec<float,9> and lie::SE23<posit32>
with det::Vec<posit32,9> (typedef posit32 = sw::universal::posit<32,2>),
checking X.log() ≈ xi with tolerance ~1e-4 via require_vec_close; also add
TEST_CASE "SE23 log is finite for a half-turn rotation" using lie::SE23<double>
with a Tan near θ≈π (use the suggested xi) and assert all entries of X.log() are
finite (std::isfinite) to exercise the SO3 inverse-left-Jacobian near its
removable singularity. Ensure test names match the pattern in the file and reuse
require_vec_close/REQUIRE as shown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b169f8d-e541-4bf4-a205-01bfeec6d54d
📒 Files selected for processing (2)
math/include/branes/math/lie/se23.hpptests/math/lie_groups.cpp
…singular coverage - The adjoint TEST_CASE name had a Unicode middle-dot which MSVC ctest codepage-mangles, so the filter matched no case and the job failed. Use ASCII. - Address CodeRabbit: add SE2(3) round-trip tests in float and posit32 (the math-lib custom-arithmetic requirement) and a near-half-turn regression that exercises the left-Jacobian-inverse removable singularity in exp/log + adjoint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/math/lie_groups.cpp`:
- Around line 478-482: The test is asserting branch-cut-sensitive equality for
the half-turn case by checking X.log() == xi; instead, ensure the log is finite
and that the group invariant holds: replace the require_vec_close(X.log(), xi,
1e-9) call with two checks—verify the components of X.log() are finite (no
NaN/inf) and verify that lie::SE23<double>::exp(X.log()) is numerically close to
X (e.g., require_mat_close(lie::SE23<double>::exp(X.log()), X, 1e-9) or
similar). Keep the rest of the loop (xi, X, dxi) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c01cd828-1c20-4191-ab6b-2e30202ae2f3
📒 Files selected for processing (1)
tests/math/lie_groups.cpp
…anch cut Address CodeRabbit: at theta = pi log is not unique, so asserting log(X) == xi can fail for a correct implementation. Check exp(log(X)) close to X instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase B foundation for the R-IEKF epic (#347).
SE₂(3)— the extended-pose / double-direct-isometry group(R, v, p)— is the state manifold that makes the IMU dynamics group-affine, so the right-invariant error Jacobians are state-independent and the unobservable yaw/position subspace is preserved by construction. That's the cure Phase A validated by measurement (flat yaw leak #349, state-independent propagationΦ#351).What's here
math/lie/se23.hpp—exp/log(both translations share theSO(3)left Jacobian), compose, inverse, 5×5 matrix, 9×9 adjoint, in the rotation-first[φ; ν; ρ]ordering. Mirrors these3.hpp/sim3.hppidioms; clean-room (Solà 2018, Barrau-Bonnabel 2017).lie_groups):exp/loground-trip, compose/inverse, and the adjoint identityX·exp(ξ)·X⁻¹ = exp(Ad_X·ξ)— 42 assertions, alongside the existing SO3/SE3/Sim3 suite (741 total, green).Next in Phase B
The invariant IMU propagation and camera measurement update on this state, then the
MsckfInvariantBackendwiring (parallel to the shipped EKF) + the EuRoC verdict.Refs #347, #348, #212.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests