-
Notifications
You must be signed in to change notification settings - Fork 870
Template EqF on Group and Manifold #2197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Template EqF on Group and Manifold #2197
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice progress! Some initial comments.
…equire for template, change EqF to take in initial state, use traits for group
73215df to
4ee2efe
Compare
examples/EquivariantFilter.h
Outdated
| Matrix Q = Geometry::processNoise(u); // Replaces blockDiag(...) | ||
|
|
||
| X_hat = traits<G>::Compose(X_hat, traits<G>::Expmap(L * dt)); | ||
| Sigma = Phi * Sigma * Phi.transpose() + Bt * Q * Bt.transpose() * dt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already done in the ManifoldEKF, specialized to groups in the LieGroupEKF, which is the main reason to derive form that class.
examples/EquivariantFilter.h
Outdated
| Vector3 delta_vec = Rot3::Hat(y.d.unitVector()) * action_result; | ||
| Matrix Dt = Geometry::outputMatrixDt(y.cal_idx, X_hat); | ||
|
|
||
| Matrix S = Ct * Sigma * Ct.transpose() + Dt * y.Sigma * Dt.transpose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll also find this somewhere in the existing hierarchy.
|
Nice to see this one in gtsam 👍🏻 |
|
Thans @AlessandroFornasier !We're going to substantially rewrite and base on LieGroupEKF (or derived), we'll let you know :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good meeting. Trying to give some more feedback here. The goal is whether we can get rid of Geometry altogether.
gtsam_unstable/geometry/ABC.h
Outdated
|
|
||
| /// Initialize the symmetry group G | ||
| G(const Rot3& A = Rot3::Identity(), const Matrix3& a = Matrix3::Zero(), | ||
| static constexpr int numSensors = N; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also be small n if we are to match the paper.
|
|
||
| // Adjoint matrix of this group element (for SE(3) or similar) | ||
| Eigen::Matrix<double, dimension, dimension> AdjointMap() const { | ||
| // TODO: implement properly for your group structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to implement for your group, or at least figure out the math, as in the invariant filter the A matrix is just
gtsam_unstable/geometry/ABC.h
Outdated
| */ | ||
| template <size_t N> | ||
| Input velocityAction(const G<N>& X, const Input& u) { | ||
| Input velocityAction(const Group<N>& X, const Input& u) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Input type is mixing two things I believe: a mathematical object, Input to InputData, and then redefine Input as simply
Using Input = Vector6
Or even just use Vector6.
| return gtsam::numericalDerivative11<Vector, G<N>>( | ||
| [&xi](const G<N>& g) { return xi.localCoordinates(g * xi); }, | ||
| gtsam::traits<G<N>>::Identity()); | ||
| return gtsam::numericalDerivative11<Vector, Group<N>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is only used in tests move it there.
| #pragma once | ||
| #include <gtsam/base/Matrix.h> | ||
| #include <gtsam/base/Vector.h> | ||
| #include <gtsam/geometry/Rot3.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These includes seem out of place when M and G are generic.
| void update(const typename Geometry::Measurement& y); | ||
|
|
||
| static constexpr int DOF = gtsam::traits<G>::dimension; | ||
| static constexpr int n_cal = Geometry::n_cal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also out of place in a generic filter.
gtsam_unstable/geometry/ABC.h
Outdated
| * @return Linearized state matrix | ||
| * Uses Matrix zero and Identity functions | ||
| */ | ||
| static Matrix stateMatrixA(const GType& X_hat, const Input& u) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually used in the filter?
| Matrix Q = Geometry::processNoise(u); | ||
|
|
||
|
|
||
| this->X_ = traits<G>::Compose(this->X_, traits<G>::Expmap(L * dt)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me think
| Vector Delta = InnovationLift * K * delta_vec; | ||
| this->X_ = traits<G>::Compose(traits<G>::Expmap(Delta), this->X_); | ||
| this->P_ = (Matrix::Identity(DOF, DOF) - K * Ct) * this->P_; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to see whether you can do this update by constructing the parameters of update in the base class. It will be informative.
gtsam_unstable/geometry/ABC.h
Outdated
|
|
||
| /// Input struct for the Biased Attitude System | ||
|
|
||
| struct Input { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would split this file up: ABC.h is pure geometry. All the other data structures (Input, etc) should be moved to the example - there should be zero “infra” code in this geometry code.
Updates to template equivariant filter on group and manifold