Conversation
|
\bench |
|
/bench |
|
/bench |
|
/bench |
1 similar comment
|
/bench |
|
Result is cached, errrrgh, I should add a command to force re-bench |
|
I don't trust the Mac benchmarks - probably was run on a different machine from cached, @ProfFan and I discussed a fix to benchmark flow (ps are we running that in release!? numbers below are much better, on an M1, with TBB). Here are my results: New:
Old:
So I think there is no regression and this can be merged. The Pose3-specific Expmap was a bit faster than without, although we should have much stronger benchmarks to really conclude that... |
varunagrawal
left a comment
There was a problem hiding this comment.
Did a first pass and there is some weird naming that raised some flags.
I am really liking the de-duplication induced, however, I am a bit skeptical about the need to support Eigen::Dynamic since I haven't found any use of
Once I get a response on the naming comments, I can finish looking at the rest of the gtsam/geometry/ExtendedPose3-inl.h file and approve.
…m, add perf note Co-authored-by: dellaert <10515273+dellaert@users.noreply.github.com>
Address PR #2418 review feedback: SWIG Jacobians, trait-based dim, AdjointMap perf note
varunagrawal
left a comment
There was a problem hiding this comment.
I believe this looks good.

A major refactor that brings in$SE_k(3)\doteq SO(3) \ltimes (\mathbb{R}^3)^k$ and eliminates a lot of duplicate code in $SE_k(3)$ can now be used in invariant filter implementations. A selection with $k={2, 3, 4, 6}$ is exposed in the wrapper as well.
Pose3andNavState. In addition,What changed
ExtendedPose3<K, Derived>ingeometry, supporting:K >= 1K = Eigen::Dynamic(runtimekconstructor)Manifold/LieGroup/MatrixLieGroupsurface.Deriveddefaulted) so derived classes keep concrete return types (compose,inverse,Expmap, etc.) without heavy wrappers.Pose3->ExtendedPose3<1, Pose3>NavState->ExtendedPose3<2, NavState>ExtendedPose3notebook with Lie-group math intro, Colab preamble tags, and Python examples.github/copilot-instructions.mdnotebook conventions.Why this is better
SE_k(3)math acrossPose3,NavState, and futurek.Tests run
C++
make -j6 testExtendedPose3.run✅make -j6 testPose3.run✅make -j6 testNavState.run✅Wrapper/build
make -j6 install✅make -j6 python-install✅make -j6 python-test✅