Skip to content

Add ObservationManager - #2575

Merged
sarlinpe merged 24 commits into
mainfrom
sarlinpe/observation-manager
May 27, 2024
Merged

sarlinpe merged 24 commits into
mainfrom
sarlinpe/observation-manager

Conversation

@sarlinpe

@sarlinpe sarlinpe commented May 24, 2024 •

Copy link
Copy Markdown
Member

This removes the filtering logic in Reconstruction as well as the SfM-specific data in Image, such as the visibility pyramid. The logic is unchanged except one minor point: I removed the num_observations column in the GUI database image viewer because this field is now only available during reconstruction. It is anyway mostly redundant with num_points2D.

@sarlinpe
sarlinpe marked this pull request as ready for review May 25, 2024 08:37
@sarlinpe
sarlinpe requested review from B1ueber2y and ahojnnes May 25, 2024 08:39

const double tri_angle = CalculateTriangulationAngle(
proj_center1, proj_center2, point3D.xyz);
EXPECT_GE(tri_angle, DegToRad(kMinTriAngleDeg));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test sometimes fails on mac and Windows with the previous value of 1°. It seems that the sampling of camera poses doesn't constrain them to have a sufficient angle.

@sarlinpe
sarlinpe enabled auto-merge (squash) May 25, 2024 17:07
Comment thread src/colmap/sfm/observation_manager.h Resolved
Comment thread src/colmap/estimators/alignment.cc Resolved
Comment thread src/colmap/sfm/incremental_mapper.cc Resolved
Comment thread src/colmap/sfm/observation_manager.cc Resolved
Comment thread src/colmap/sfm/observation_manager.cc Resolved
@ahojnnes

Copy link
Copy Markdown
Contributor

Looks great to me. I assume most of the code in the ObservationManager is untouched and just moved from Reconstruction/Image classes? Have you run through a dataset and checked that it produces the exact same number of final observations?

@sarlinpe

Copy link
Copy Markdown
Member Author

PTAL. I have indeed mostly moved around code without change of logic. I have checked that the result is identical in terms of reconstruction statistics.

@sarlinpe
sarlinpe requested a review from ahojnnes May 26, 2024 21:46
@sarlinpe
sarlinpe disabled auto-merge May 27, 2024 07:04
@sarlinpe
sarlinpe enabled auto-merge (squash) May 27, 2024 07:05
@ahojnnes

Copy link
Copy Markdown
Contributor

Perfect, thanks a lot.

@sarlinpe
sarlinpe merged commit eaebe3c into main May 27, 2024
@sarlinpe
sarlinpe deleted the sarlinpe/observation-manager branch May 27, 2024 07:27
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 9, 2026
## Summary

Update the visibility pyramid when a feature gains its first triangulated correspondence or loses its last. Additional correspondences change support but leave the visible feature set unchanged, so repeating every pyramid-level update does unnecessary work.

## Changes

- Put SetPoint and ResetPoint beside the existing zero/one visibility-count transitions, preserving occupied-cell scores for colliding features.
- Extend the existing visibility-score and AddImage tests to cover repeated support, collisions, partial/final removal, underflow and rebuilding the manager.

## Background

The repeated updates are inherited from fe8f82a, "Initial commit" (2016), in src/base/image.cc. The ObservationManager extraction, eaebe3c, "Add ObservationManager (colmap#2575)", retained them.

## Results

Fixed public Add64/Delete32/Delete32 bookkeeping on a 16-image, 64-track, 120-pair fixture:

| Platform | Before | After |
| --- | ---: | ---: |
| Linux/WSL | 1.395 ms | 0.640 ms |
| Windows | 2.776 ms | 1.438 ms |

Each value averages four fresh process means per arm, ABBAABBA, with two warmup and ten measured batches per process. Canonical inputs and outputs match within each platform.

Whole-command Linux observations, including setup/oracles/serialization: median CPU 0.495 -> 0.443 s, median per-process peak RSS 414.50 -> 414.35 MiB, mean instructions 3.879 -> 3.782 billion. Windows median CPU 2.234 -> 2.211 s and median per-process peak working set 14.54 -> 14.33 MiB have overlapping process ranges. These broader counters describe the probe, not isolated pyramid operations.

## Validation

- Current composition: all 18 maintained manager cases and two mapper lifecycle cases passed, without skips. Earlier matched arms passed the same selected counts on Linux and Windows.
- clang-tidy-18 completed on the owner and maintained test with no displayed source diagnostics; generated/header warnings were suppressed. Changed-line formatting passed with the real source filename.
- Incremental build: three translation units and two test links, reusing 46 production objects and 13 archives. Source, selected-link and provider postflight checks completed using the retained outputs.

AI disclosure: Totally vibe coded
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 9, 2026
## Summary

Update the visibility pyramid when a feature gains its first triangulated correspondence or loses its last. Additional correspondences change support but leave the visible feature set unchanged, so repeating every pyramid-level update does unnecessary work.

## Changes

- Put SetPoint and ResetPoint beside the existing zero/one visibility-count transitions, preserving occupied-cell scores for colliding features.
- Extend the existing visibility-score and AddImage tests to cover repeated support, collisions, partial/final removal, underflow and rebuilding the manager.

## Background

The repeated updates are inherited from fe8f82a, "Initial commit" (2016), in src/base/image.cc. The ObservationManager extraction, eaebe3c, "Add ObservationManager (colmap#2575)", retained them.

## Results

On the composed-fork version, fixed public Add64/Delete32/Delete32 bookkeeping on a 16-image, 64-track, 120-pair fixture:

| Platform | Before | After |
| --- | ---: | ---: |
| Linux/WSL | 1.395 ms | 0.640 ms |
| Windows | 2.776 ms | 1.438 ms |

Each value averages four fresh process means per arm, ABBAABBA, with two warmup and ten measured batches per process. Canonical inputs and outputs match within each platform.

Whole-command Linux observations, including setup/oracles/serialization: median CPU 0.495 -> 0.443 s, median per-process peak RSS 414.50 -> 414.35 MiB, mean instructions 3.879 -> 3.782 billion. Windows median CPU 2.234 -> 2.211 s and median per-process peak working set 14.54 -> 14.33 MiB have overlapping process ranges. These broader counters describe the probe, not isolated pyramid operations.

## Validation

- Clean base and candidate: all 15 maintained manager cases passed in each arm, without skips. The composed-fork version also passed 18 manager and two mapper cases on Linux and Windows.
- Configured clang-tidy-18 checked observation_manager.cc in both arms and the maintained test, with no displayed source diagnostics; generated/header warnings were suppressed. Changed-line formatting passed.
- Incremental qualification reused retained pristine-upstream support and rebuilt the two selected RANSAC-dependent owners needed for the newer headers. Final source, selected-link and runtime-provider checks passed.

AI disclosure: Totally vibe coded
ahojnnes pushed a commit that referenced this pull request Nov 10, 2026
## Summary

Update the visibility pyramid only when an image feature gains its first
triangulated correspondence or loses its last. Extra supporting
correspondences leave occupied cells unchanged, so repeating every
pyramid-level update does unnecessary work.

This is a small inherited optimization, based directly on official
COLMAP 3c1a1b9.

## Changes

- Move SetPoint/ResetPoint into the existing zero/one visibility
transitions.
- Extend the maintained score test with repeated support, distinct
colliding features, partial/final removal and underflow checks.
- Extend AddImage with observation removal and reconstruction of the
manager, avoiding unrelated fork registration behavior.

## Background

The update pattern dates to fe8f82a,
"Initial commit" (2016), in src/base/image.cc. It was retained by
eaebe3c, "Add ObservationManager
(#2575)".

## Results

The source-equivalent transition change was measured in a composed-fork
fixed bookkeeping probe: Add64/Delete32/Delete32 on 16 images, 64 tracks
and 120 pairs.

| Platform | Before | After |
| --- | ---: | ---: |
| Linux/WSL | 1.395 ms | 0.640 ms |
| Windows | 2.776 ms | 1.438 ms |

Each table value is the arithmetic mean of four fresh process means per
arm (ABBAABBA). Each process mean averages ten measured batches after
two warmups. Canonical inputs and outputs agree within each platform.
These are fixed bookkeeping timings, not reconstruction throughput
measurements.

## Validation

- Independent source review accepted the clean-base production change
and adapted lifecycle fixture.
- Clean base and candidate each passed all 15 maintained manager cases,
without skips.
- Configured clang-tidy-18 checked observation_manager.cc in both arms
and the maintained test with no displayed source diagnostics;
generated/header warnings were suppressed.
- Reused retained pristine-upstream support and rebuilt the two selected
consumers of newer RANSAC headers; final source, link and provider
checks passed.
- Separately, the composed-fork version passed 18 manager and two mapper
cases on Linux and Windows.

AI disclosure: Totally vibe coded
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 10, 2026
## Summary

Integrate the upstream visibility-transition optimization into the fork while retaining its observation-manager behavior and both lifecycle checks.

## Changes

- Merge upstream 137f216, "SfM: Update visibility on feature transitions (CPU perf) (colmap#5006)", as the second parent; retain fork d896761 and its current production implementation.
- Exercise the upstream partial and final observation-removal sequence and the fork registration and deregistration sequence with independently constructed state in the existing AddImage test. Preserve the remaining fork tests.
- Attribute the inherited pattern to fe8f82a, "Initial commit", and eaebe3c, "Add ObservationManager (colmap#2575)".

## Validation

- The selected Linux observation-manager suite passed 18/18 cases with no skips; the AddImage case executed both independent sequences. The changed test and current reconstruction owner were rebuilt, and selected archive members, source dependencies and runtime providers were checked.
- Configured clang-tidy18 on the changed test: no displayed source diagnostics; generated/header warnings suppressed, consistent with retained baseline. Clang-format 18.1.8 left the affected AddImage range unchanged.

AI disclosure: Totally vibe coded
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