ethz-asl / kalibr

The Kalibr visual-inertial calibration toolbox
Other
4.41k stars 1.41k forks source link

rovio config exporter extrinics #161

Closed NikolausDemmel closed 6 years ago

NikolausDemmel commented 6 years ago

There might be a bug in the rovio config exporter.

So here we read T_cam_imu (trafo from imu to camera frame) from the Kalibr config file and invert, giving us T_SC and q_SC (trafo from camera to imu frame, i.e. camera position and orientation in imu frame).

https://github.com/ethz-asl/kalibr/blob/b95f5b83583e0a0cd1b06a4e50b9205a464170ba/aslam_offline_calibration/kalibr/python/exporters/kalibr_rovio_config#L112-L114

Later we assign this to variables qCM and MrMC from the rovio config file.

https://github.com/ethz-asl/kalibr/blob/b95f5b83583e0a0cd1b06a4e50b9205a464170ba/aslam_offline_calibration/kalibr/python/exporters/kalibr_rovio_config#L33-L36

Judging from the notation and also the rovio IJRR paper, it seems that while MrMC matches the input (camera position in imu frame), qCM does not match, as it represents the imu orientation in camera frame (i.e. q_SC inverted).

Is this a bug or did I miss something?

ZacharyTaylor commented 6 years ago

There is a bit of a mess in this converter. I can't remember which but one of rovio, kalibr or some python math functions use JPL quaternions while the others use Hamilton. Hence the seemingly random inversions

NikolausDemmel commented 6 years ago

Rovio uses Hamilton (it says so in the comment linked above).

Indeed if there was a conversion from JPL to Hamilton, then the result would be correct, albeit the variable names and comment very confusing.

Are we sure that Kalibr uses JPL?

See also: http://fzheng.me/2017/11/12/quaternion_conventions_en/

NikolausDemmel commented 6 years ago

Might indeed be JPL:

https://github.com/ethz-asl/kalibr/blob/b95f5b83583e0a0cd1b06a4e50b9205a464170ba/Schweizer-Messer/sm_kinematics/src/quaternion_algebra.cpp#L77-L101

ZacharyTaylor commented 6 years ago

sm and aslam which it uses do, here is an old discussion of this issue with different representations being used in our lab https://github.com/ethz-asl/ethzasl_msf/issues/19

HannesSommer commented 6 years ago

Yes, unfortunately sm is still using JPL's convention, including their flipped quaternion multiplication, which is the root problem here. That is why internally it is considered outdated where replacements are https://github.com/ethz-asl/minkindr/tree/master/minkindr or https://github.com/ethz-asl/kindr (both Hamilton).

@NikolausDemmel, thanks for the link to http://fzheng.me/2017/11/12/quaternion_conventions_en/ -- I wasn't aware of it, despite being very much interested in that topic.

ZacharyTaylor commented 6 years ago

Also the comments in the code could well be inconsistent, most of these conversion scripts were originally just hacked together for quick internal use in the ASL rotary wing group. However they were useful enough to warrant putting in the main repo, though we didn't do much of a cleanup before pushing them.

NikolausDemmel commented 6 years ago

They are definitely useful and I'm certainly not blaming anyone. I know how this typically goes and I am very grateful for Kalibr and other excellent OSS projects from ASL. Thanks!

Would a PR with updated comments be helpful?

NikolausDemmel commented 6 years ago

@HannesSommer I just found this link while googling just then. It is an interesting summary of the usage of Hamilton vs JPL in various popular projects. I'm not sure I can agree with the "hate" towards JPL though (it reads like the author is also not seriously cursing them, but more annoyed at the current situation).

Even if it is not from great importance, I'm still a bit curious about the history around these conventions and it would be nice to see a copy of the apparently often cited "Quaternions - Proposed Standard Conventions" tech report. Might very well be that JPL used quaternions before there even was open source software :-). Maybe this is a bit like complaining about geodetic engineers using left-handed coordinate systems...

By the way, I agree with the article about people often not being aware of the different convention. Actually, often it is cited as an advantage of Quaternions over Euler angles even in publications that for Euler angles you have to always specify which convention you use 🙈 .

HannesSommer commented 6 years ago

@NikolausDemmel , sorry for the delay. Very good point with the lost advantage over Euler angles! :). This "Quaternions - Proposed Standard Conventions" never got published by JPL as far as I know Therefore it is only available from withing JPL (and probably NASA) :(. But it is not the publication where this was "introduced". Instead this one is probably most responsible (even according to Shuster himself) for the adoption of the flipped multiplication (called natural order by the author):

Shuster, Malcolm D. "A survey of attitude representations." Navigation 8.9 (1993): 439-517.

Personally I'm quite angry with the responsible people at JPL, because better alternatives were known already back then (I even believe the Space Shuttle program was doing it "right"! - while the ISS is not anymore) and I'm pretty sure they knew to some degree what they were doing. In fact I'm angry enough for trying hard to accelerate the process of finding out of this conventional dead-end again (at least for robotics) with this not yet published paper: https://www.researchgate.net/publication/323426570_Why_and_How_to_Avoid_the_Flipped_Quaternion_Multiplication_-_Shorter_and_Less_Formal_Version (long version: https://arxiv.org/abs/1801.07478) (sorry for the advertisement -- I hope and believe that the purpose justifies it)

NikolausDemmel commented 6 years ago

This is really interesting, but not super urgent, so no worries about the delay. I will have to read your paper, thanks for sharing (@schneith linked to it already the other day over at https://github.com/ethz-asl/maplab/issues/46#issuecomment-367972276). I might get back to you after the read :wink:.