artivis / manif

A small C++11 header-only library for Lie theory.
https://artivis.github.io/manif
MIT License
1.46k stars 239 forks source link

Update ceres::LocalParameterization to ceres::Manifold #295

Closed mattalvarado closed 2 months ago

mattalvarado commented 2 months ago

Ceres v2.2.0 has deprecated ceres::LocalParametrization in favor of ceres::Manifold. This PR updates the CeresManifoldFunctor to match the new expected interface for the ceres::Manifold.

Ubuntu 24.04 seems to require Ceres to be updated to v2.2.0 when building Ceres from source.

Edit: Closes https://github.com/artivis/manif/issues/288 Closes https://github.com/artivis/manif/issues/290 Closes https://github.com/artivis/manif/issues/296

artivis commented 2 months ago

Hi @mattalvarado, Thanks for opening this PR!

As you can see, manif supports several Ubuntu versions going back to 20.04. It should thus also support Ceres both pre and post 2.2. Would you try to do that based on the Ceres version detected? Ceres defines the macros CERES_VERSION_MAJOR/MINOR/REVISION in ceres/version.h. Using some if/else pre-processor directives everywhere necessary should do the trick.

artivis commented 2 months ago

@mattalvarado I've adressed my own comments above and opened a PR against your own fork to let you update your PR if it all looks fine to you. See the PR here. If the change looks fine, please merge that PR and I'll merge this one once updated.

mattalvarado commented 2 months ago

@mattalvarado I've adressed my own comments above and opened a PR against your own fork to let you update your PR if it all looks fine to you. See the PR here. If the change looks fine, please merge that PR and I'll merge this one once updated.

Thanks @artivis! Your PR is merged

artivis commented 2 months ago

Closes #288 Closes #290 Closes #296

artivis commented 2 months ago

Thanks a lot @mattalvarado for the great contribution! The CI is passing now, merging :)