Skip to content

Left invariant EKF header file, with an example on SE(2) and NavState#2113

Closed
scottiyio wants to merge 10 commits intoborglab:developfrom
scottiyio:develop
Closed

Left invariant EKF header file, with an example on SE(2) and NavState#2113
scottiyio wants to merge 10 commits intoborglab:developfrom
scottiyio:develop

Conversation

@scottiyio
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!
Like we discussed, examples need docs. Not doxygen, which is for APIs, but a bit more. Look at other examples.
Maybe change methods "getFooBar" to just "fooBar".
Please format all files with clang-format.

Vector9 result;
result << w, Z_3x1, a;
if (H) {
*H = Matrix::Zero(9, 9);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H->setZero() will be faster.
These dynamics don't depend on the state so maybe we have to rethink this. No sense in multiplying with a bunch of zeros. Let's talk in the meeting.

return result;
}

// define a GPS measurement processor. The GPS measurement processor returns
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

measurement function, not processor.

// define a GPS measurement processor. The GPS measurement processor returns
// the expected measurement h(x) = translation of X with a Jacobian H used in
// the update stage of the LIEKF.
Vector3 h_gps(const NavState& X, OptionalJacobian<3, 9> H = {}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to add this to NavState and properly unit test


// Create the filter with the initial state, covariance, and dynamics and
// measurement functions.
GeneralLIEKF<NavState, Vector3, 6> ekf(X0, P0, dynamics_func, h_func);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just pass dynamics and h_gps here? Ask the AI :-)

// Create the IMU measurements of the form (linear_acceleration,
// angular_velocity)
Vector6 imu1, imu2;
imu1 << 0.0, 0.0, 0.0, 0.0, 0.0, 0.0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was good for compiling but now we need values?

* @tparam Measurement Type of measurement (e.g. Vector3 for a GPS measurement
* for 3D position)
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kill extra blank line

}

protected:
LieGroup X; ///< Current state estimate.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private or protected members take underscore

Matrix S = H * P * H.transpose() + R;
Matrix K = P * H.transpose() * S.inverse();
X = X.expmap(-K * y);
P = (I_n - K * H) * P; // move Identity to be a constant.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove todo

I_n; ///< A nxn identity matrix used in the update stage of the LIEKF.
};

/**
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs no doxygen, just say what this is in one-liner comment.

* @tparam _p The dimension of the control vector.
*/
template <typename LieGroup, typename Measurement, size_t _p>
class GeneralLIEKF : public LIEKF<LieGroup, Measurement> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to think that rather than subclassing the filter, we should instead have different types of dynamics. Not sure how yet, let's chat.

@dellaert dellaert mentioned this pull request Apr 26, 2025
@dellaert
Copy link
Copy Markdown
Member

Closing as superseded by #2115

@dellaert dellaert closed this Apr 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants