search

cartographer-project / cartographer

Cartographer is a system that provides real-time simultaneous localization and mapping (SLAM) in 2D and 3D across multiple platforms and sensor configurations.
Apache License 2.0
7.03k stars 2.24k forks source link

Use Manifold instead of LocalParameterization. #1882

Open zjwoody opened 2 years ago

zjwoody commented 2 years ago

We need to migrate the Cartographer code to use Manifolds instead of LocalParameterization to allow Ceres code to remove the LocalParameterization code.

The PR converts internal use of LocalParameterization to Manifold. Removes headers that reference local_parameterization or implementation classes.

Note that internal variable and method names with type Manifold were renamed Manifold/manifold, but the outer abstraction class, proto name and so on were left as Parameterization/parameterization.

Signed-off-by: Jie Zheng zhengj@google.com

wohe commented 1 year ago

@zjwoody Sorry, it took slightly longer than expected. Now CI should work again. Can you rebase your PR? I also had a quick look at it already, and my understanding is that it will not compile as is:

  1. We support Ubuntu and Debian releases until LTS ends, so for several years we will have to be able to use a Ceres version which does not yet support Manifold.
  2. Thus, we would need to guard using a version number as suggested in #1879.
  3. Just FYI, this should not delay this PR in any way. If I understand correctly then both Ubuntu 22.04 and Debian Bullseye do not support Manifold, so CI would not be able to test the Manifold related code. But it seems the following releases when they happen will pick it up. Probably 2024 if Ubuntu releases as usual, and whenever Debian does.
zjwoody commented 1 year ago

Hi Wolfgang, thanks for the information! I rebased the PR and it is not able to compile due to the reason you mentioned, so it can't be merged. I have some stupid questions since I'm not clear about what to do next:

  1. For the guard of ceres version number, do you mean limit the ceres version used in Cartographer to the old version(e.g. < 2.1.0) before it switched to the manifold?
  2. If 1 is true and we guard the ceres version, what should we do with this pr? Thanks!
wohe commented 1 year ago

Sorry about the slow response. I was referring to the suggestion from #1879. Basically, you have to put two versions of the code in some places like this, checking whether a recent Ceres version is used:

#if CERES_VERSION_MAJOR > 2 || CERES_VERSION_MAJOR == 2 && CERES_VERSION_MINOR >= 1
// TODO: new code here Manifold
#else
// TODO: old code here LocalParameterization
#endif

You might have to #include "ceres/version.h". Does that make sense?