Skip to content

Feature sub-selection for global BA #3650

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.

Sign up for GitHub

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

Jump to bottom
Merged
ahojnnes merged 16 commits into from
Oct 20, 2025
Merged

Feature sub-selection for global BA #3650

ahojnnes merged 16 commits into from
Oct 20, 2025

Conversation

@ahojnnes
Copy link
Contributor

@ahojnnes ahojnnes commented Oct 2, 2025 •
edited
Loading

Depending on the size of the model, leads to speedups of the end-to-end runtime by ~2x by removing around 80-90% of the points from the BA problem with little impact on the final model. Not tested much so far.

Benchmarking results with default params on ETH3D benchmark against current main branch:

I20251003 13:56:23.288329 140419113386112 compare.py:main:58] Results A - B:
=====scenes===== ======AUC @ X deg (%)====== ===images=== =components=
                  0.5    1.0    5.0    10.0     reg   all  num largest

==============================eth3d=dslr==============================
botanical_garden  -0.04   0.64   0.31   0.16      0     0    0       0
boulders           1.32   1.72   0.95   0.63      0     0    0       0
bridge             0.38   0.21   0.05   0.03      0     0    0       0
courtyard          0.96   1.36   0.49   0.25      0     0    0       0
delivery_area     -0.75  -0.42  -0.08  -0.04      0     0    0       0
door               0.08  -2.12  -0.49  -0.24      0     0    0       0
electro            0.44   0.38   0.13   0.06      0     0    0       0
exhibition_hall   -1.96  -1.06  -0.36  -0.19      0     0    0       0
facade            -0.44  -0.30  -0.06  -0.03      0     0    0       0
kicker             0.78  -0.52  -0.01  -0.01      0     0    0       0
lecture_room       0.43   0.18   0.13   0.07      0     0    0       0
living_room       30.20  34.56  38.40  38.95      0     0   -1       0
lounge             0.00   0.00   0.02   0.01      0     0    0       0
meadow            -1.14  -1.01  -0.20  -0.10      0     0    0       0
observatory        0.52  -0.20  -0.11  -0.05      0     0    0       0
office            15.28  19.24  27.20  29.89      0     0   -1       0
old_computer       1.53   0.89   0.25   0.13      0     0    0       0
pipes              3.89   2.51   0.56   0.28      0     0    0       0
playground         7.03   7.77   8.39   8.46      1     0   -1       1
relief             1.40   0.92   0.23   0.11      0     0    0       0
relief_2          -4.24  -2.32  -0.43  -0.22      0     0    0       0
statue             0.64   0.32   0.06   0.03      0     0    0       0
terrace           -0.75  -0.35  -0.07  -0.04      0     0    0       0
terrace_2          1.08   0.57   0.18   0.09      0     0    0       0
terrains          -4.55  -4.22  -1.21  -0.65      0     0    0       0
----------------------------------------------------------------------
overall            3.01   3.55   4.16   4.27      1     0   -3       1
----------------------------------------------------------------------
average            2.08   2.35   2.97   3.10      0     0    0       0

@ahojnnes ahojnnes marked this pull request as ready for review October 14, 2025 10:03
B1ueber2y
B1ueber2y reviewed Oct 15, 2025
View reviewed changes
Copy link
Contributor

@B1ueber2y B1ueber2y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The tiling and coverage gain make sense to me and are probably the best we can do without prior 3D poses.

I am wondering if it makes sense to support this feature in BundleAdjustmentController as well.

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Oct 17, 2025

Thanks. The tiling and coverage gain make sense to me and are probably the best we can do without prior 3D poses.

I am wondering if it makes sense to support this feature in BundleAdjustmentController as well.

Good suggestion. I refactored it, so this now becomes a feature of the bundle adjuster itself.

B1ueber2y
B1ueber2y reviewed Oct 18, 2025
View reviewed changes
@ahojnnes ahojnnes force-pushed the user/joschonb/view-graph-pruning branch from 989904b to 8097371 Compare October 20, 2025 07:39
@ahojnnes
Copy link
Contributor Author

ahojnnes commented Oct 20, 2025

Thanks. The tiling and coverage gain make sense to me and are probably the best we can do without prior 3D poses.

I am wondering if it makes sense to support this feature in BundleAdjustmentController as well.

I'll skip this for now. We can add it later, if a need arises.

B1ueber2y
B1ueber2y approved these changes Oct 20, 2025
View reviewed changes
Copy link
Contributor

@B1ueber2y B1ueber2y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ahojnnes ahojnnes merged commit eee1d07 into main Oct 20, 2025
13 checks passed
@ahojnnes ahojnnes deleted the user/joschonb/view-graph-pruning branch October 20, 2025 17:04
S-o-T
S-o-T reviewed Oct 28, 2025
View reviewed changes
src/colmap/sfm/incremental_mapper.cc
constexpr size_t kMinNumRegFramesForFastBA = 10;
const bool use_fast_ba =
reconstruction_->NumRegFrames() >= kMinNumRegFramesForFastBA;
if (use_fast_ba) {
Copy link
Contributor

@S-o-T S-o-T Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bug, because use_fast_ba implies relaxed tolerances and iterations limits (currently the opposite is happening).

Copy link
Contributor Author

@ahojnnes ahojnnes Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Thank you.

Copy link
Contributor Author

@ahojnnes ahojnnes Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See: #3677

@BrewTestBot BrewTestBot mentioned this pull request Nov 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@B1ueber2y B1ueber2y B1ueber2y approved these changes

@sarlinpe sarlinpe Awaiting requested review from sarlinpe

+1 more reviewer

@S-o-T S-o-T S-o-T left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ahojnnes @S-o-T @B1ueber2y