Implement SE23 matrix Lie group for extended poses#40
Conversation
contagon
left a comment
There was a problem hiding this comment.
This looks awesome! I appreciate the PR and work done - it's great to see others interested in fact-rs:)
I made a few comments, should mostly be minor and easy changes. My only big picture thought is if there's a better name - SE23 is close to "SE twenty-three". I generally pronounce it "SE - 2 - 3". SE_2_3 is a bit verbose, SE_23 has the same twenty-three issue, and I worry SE2_3 is too close to SE2. So I suppose SE23 is fine unless you can think of any alternatives.
Did you have any long-term plans for using it for any residuals or anything?
| pub type Vector4<T = dtype> = na::SVector<T, 4>; | ||
| pub type Vector5<T = dtype> = na::SVector<T, 5>; | ||
| pub type Vector6<T = dtype> = na::SVector<T, 6>; | ||
| pub type Vector9<T = dtype> = na::SVector<T, 9>; |
There was a problem hiding this comment.
This is great - would you mind adding type aliases for 7 and 8 as well? And for all of the types you added 9 to in this matrix as well?
There was a problem hiding this comment.
I added them to the types 9 has been added to, as well as the matrices with one and two rows so that they are there for 1D or 2D residuals.
| fn vee(xi: MatrixView<5, 5, T>) -> Vector9<T> { | ||
| // This bypasses the missing macros for Vector9, but probably not ideal | ||
| let rotuvw = Vector6::new( | ||
| xi[(0, 3)], | ||
| xi[(1, 3)], | ||
| xi[(2, 3)], | ||
| xi[(0, 3)], | ||
| xi[(1, 3)], | ||
| xi[(2, 3)], | ||
| ); | ||
| let xyz = Vector3::new(xi[(0, 4)], xi[(1, 4)], xi[(2, 4)]); | ||
|
|
||
| let mut xi = Vector9::zeros(); | ||
| xi.as_mut_slice()[0..6].copy_from_slice(rotuvw.as_slice()); | ||
| xi.as_mut_slice()[6..9].copy_from_slice(xyz.as_slice()); | ||
|
|
||
| xi | ||
| } |
There was a problem hiding this comment.
Try using Vector9<T>::from_column_slice to initialize from a slice instead. Not sure if it reduces copies at all, but the code will at least be a bit cleaner
There was a problem hiding this comment.
Yeah, that looks way nicer
Thanks for the review! I will look into the requested changes and accommodate then :) I agree the name doesn't necessarily lend itself well to code. In the old GTSAM fork by Brossard, he used the name We informally talk about "Extended poses", but I guess it also doesn't match all that well when the rest of the Lie groups have names like As for long-term plans, right now this is mostly a hobby project with my research being focused on phased-array radio systems-aided INS using FGO, where I use C++ with GTSAM. I would however like to demonstrate feasibility of Rust-based FGO on my data just as a proof-of-concept, so residuals for range, azimuth, and elevation are ones I envision using fact-rs with, though I have mostly used fixed-lag smoothed iSAM2. |
|
Hmm, I'll keep pondering the name - I think I might end up with ExtendedSE3 or VelSE3 in the long run. Awesome! I'm mostly a gtsam user myself. Let me know if you hit any bugs or issues with fact-rs, I'd love to battle test it more fully. I've had a busy couple of weeks, but should be more responsive moving forward. Thanks again! |
Hi! As a researcher who works with factor graph optimisation applied to autonomous systems, and who wants to see Rust applied more in the robotics domain, this project seems very promising. Therefore I wanted to contribute!
This pull request implements the SE_2(3) matrix Lie group from e.g., [1] into fact-rs.
Notes about implementation
vee-implementation, I have created rotation and velocity as a Vector6 and position as Vector3 as a result, before transferring them to a Vector9 since Vector9::new is not a defined macro. Probably not optimal, so pointing it out if you have suggestions on how to do it instead.[1] Axel Barrau, Silvere Bonnabel. A Mathematical Framework for IMU Error Propagation with Appli-
cations to Preintegration. IEEE International Conference on Robotics and Automation (ICRA), May
2020, Paris, France. 10.1109/ICRA40945.2020.9197492. hal-02394774v3