8000 Modernized bundle adjustment interface by ahojnnes · Pull Request #2896 · colmap/colmap · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Modernized bundle adjustment interface #2896

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

Merged
merged 18 commits into from
Nov 17, 2024
Merged

Modernized bundle adjustment interface #2896

merged 18 commits into from
Nov 17, 2024

Conversation

ahojnnes
Copy link
Contributor
@ahojnnes ahojnnes commented Nov 7, 2024
  • Defines a single consistent abstract interface for bundle adjustment.
  • Reuses functionality between different bundle adjusters using composition rather than inheritance.
  • Extracts the pose prior alignment to the alignment module. Ideally needs tests but this remains a TODO.
  • The problem setup is now performed when the bundle adjuster is created and, as such, disentangled from the solving of the problem. This renders the current awkward SetUpProblem unnecessary.
  • The idea later is then to pass a bundle adjuster object to a simplified covariance estimation interface.

@B1ueber2y
Copy link
Contributor

Thanks! The general idea is fantastic. Will try to find time later this week to look into details.

@B1ueber2y
Copy link
Contributor
B1ueber2y commented Nov 14, 2024

Thanks. Looks great! The pybind here needs to be updated: https://github.com/colmap/colmap/blob/main/src/pycolmap/estimators/bundle_adjustment.cc#L144-L164. Also the example runner here: https://github.com/colmap/colmap/blob/main/pycolmap/examples/custom_bundle_adjustment.py#L196-L208

@ahojnnes
Copy link
Contributor Author

@B1ueber2y This should be ready for a final review. Thanks.

@B1ueber2y
Copy link
Contributor

https://github.com/colmap/colmap/blob/main/pycolmap/examples/custom_bundle_adjustment.py#L196-L208

Thanks. Should we update the runner here as well: https://github.com/colmap/colmap/blob/main/pycolmap/examples/custom_bundle_adjustment.py#L196-L208? I expect this part of code to be broken with the new interface?

@ahojnnes
Copy link
Contributor Author
ahojnnes commented Nov 17, 2024

@B1ueber2y I fixed the custom_bundle_adjustment.py code with the new interface. The idea is that the bundle adjustment problem is configured purely through the config object instead of providing access to the internal methods. One still has full control over adding custom residuals to the existing problem. I verified that this works end-to-end together with the custom_incremental_mapper.py (for which I added the option to directly call it from the command-line to process a dataset).

@B1ueber2y
Copy link
Contributor
B1ueber2y commented Nov 17, 2024

The custom_bundle_adjustment.py script wont work if pyceres is not installed, since the solver options wont parse here: https://github.com/colmap/colmap/blob/user/jsch/ba-interface/pycolmap/examples/custom_bundle_adjustment.py#L40-L45, which will result in:

TypeError: Unable to convert function return value to a Python type! The signature was
(self: pycolmap._core.BundleAdjustmentOptions) -> ceres::Solver::Options

So I added "import pyceres" back to avoid confusion when the user tries the code without pyceres installed.

8000
@B1ueber2y
Copy link
Contributor
B1ueber2y commented Nov 17, 2024

Also It might be helpful for stabilizing pycolmap interfaces if we added the example script (and potentially other python tests) to the CI pipeline.

@ahojnnes
Copy link
Contributor Author

Also It might be helpful for stabilizing pycolmap interfaces if we added the example script (and potentially other python tests) to the CI pipeline.

Yes, agreed. On my TODO list. My idea would be to expose the colmap/scene/synthetic.h interface, so we can have equivalent end-to-end mapping tests as we have under colmap/controllers/incremental_pipeline_test.cc. Later, the synthetics also come in handy for implementing tests for other parts of the python interface.

@ahojnnes ahojnnes merged commit d065cea into main Nov 17, 2024
16 checks passed
@ahojnnes ahojnnes deleted the user/jsch/ba-interface branch November 17, 2024 14:57
B1ueber2y added a commit that referenced this pull request Dec 4, 2024
…t pyceres (#2985)

Relevant discussion here: #2978

No need for ``import ceres`` in ``custom_bundle_adjustment.py`` anymore
(relevant discussion: #2896)

---------

Co-authored-by: Johannes Schönberger <jsch@demuc.de>
ahojnnes added a commit that referenced this pull request Dec 6, 2024
…t pyceres (#2985)

Relevant discussion here: #2978

No need for ``import ceres`` in ``custom_bundle_adjustment.py`` anymore
(relevant discussion: #2896)

---------

Co-authored-by: Johannes Schönberger <jsch@demuc.de>
HernandoR pushed a commit to HernandoR/colmap that referenced this pull request Dec 30, 2024
…t pyceres (colmap#2985)

Relevant discussion here: colmap#2978

No need for ``import ceres`` in ``custom_bundle_adjustment.py`` anymore
(relevant discussion: colmap#2896)

---------

Co-authored-by: Johannes Schönberger <jsch@demuc.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0