Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 10 commits into
base: devel
Choose a base branch
from

Conversation

mattalvarado
Copy link

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.

@artivis artivis added the enhancement New feature or request label Jun 21, 2024
@artivis artivis self-assigned this Jun 21, 2024
@artivis
Copy link
Owner

artivis commented Jun 27, 2024

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 artivis mentioned this pull request Jun 27, 2024
@artivis
Copy link
Owner

artivis commented Jun 27, 2024

@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.

Support Ceres both pre/post 2.2
@mattalvarado
Copy link
Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants