Skip to content

Add max_num_features option to automatic_reconstructor and benchmark - #4907

Merged
ahojnnes merged 1 commit into
mainfrom
sarlinpe/max-num-features
Oct 30, 2026
Merged

ahojnnes merged 1 commit into
mainfrom
sarlinpe/max-num-features

Conversation

@sarlinpe

@sarlinpe sarlinpe commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds a max_num_features option to automatic_reconstructor and to the reconstruction benchmark, to evaluate a feature type with a different number of keypoints than its default. The first use is evaluating learned matchers at 2k/4k/8k keypoints.

Changes

  • AutomaticReconstructionController::Options::max_num_features (default -1, off).
    • When positive, it overrides max_num_features of the SIFT, ALIKED and LoMa extraction options.
    • It is applied after the quality preset, which otherwise sets the number of SIFT features.
  • colmap automatic_reconstructor --max_num_features N.
  • Benchmark: benchmark/reconstruction/evaluate.py --max_num_features N forwards the option to every automatic_reconstructor call (evaluation/utils.py).

Validation

  • With --feature loma --max_num_features {2048, 4096, 8192}, all 898 images of ETH3D DSLR have exactly 2048 / 4096 / 8192 keypoints.
  • With the option unset, behaviour is unchanged.
  • controllers/ tests pass.

Part of the FlexGlue stack: #4924, then this PR, #4921, #4908 and #4915. #4908 uses it for its evaluation.

@sarlinpe
sarlinpe added this pull request to stack #4909 October 27, 2026 22:22
@sarlinpe
sarlinpe requested a review from ahojnnes October 27, 2026 23:23
@sarlinpe
sarlinpe removed this pull request from stack #4909 October 28, 2026 07:45
@sarlinpe
sarlinpe added this pull request to stack #4917 October 28, 2026 07:46
@sarlinpe
sarlinpe enabled stacked auto-merge (squash) October 28, 2026 20:34
@sarlinpe
sarlinpe force-pushed the sarlinpe/max-num-features branch from f44de67 to ab6adb4 Compare October 29, 2026 05:43
stacked auto-merge was automatically disabled October 29, 2026 05:57

Pull request removed from stack

@sarlinpe
sarlinpe removed this pull request from stack #4917 October 29, 2026 05:57
@sarlinpe
sarlinpe added this pull request to stack #4922 October 29, 2026 05:57
@sarlinpe sarlinpe added the full-ci Run Windows and remaining pycolmap CI label Oct 29, 2026
@sarlinpe
sarlinpe force-pushed the sarlinpe/max-num-features branch from ab6adb4 to 51a39d9 Compare October 29, 2026 23:27
@sarlinpe
sarlinpe removed this pull request from stack #4922 October 29, 2026 23:28
@sarlinpe
sarlinpe changed the base branch from main to sarlinpe/loma-bf16-probe October 29, 2026 23:28
@sarlinpe
sarlinpe added this pull request to stack #4925 October 29, 2026 23:28
Base automatically changed from sarlinpe/loma-bf16-probe to main October 30, 2026 08:37
Overrides the maximum number of features per image of the selected feature type
(SIFT, ALIKED, LoMa), e.g. to evaluate learned features with more keypoints than
the default.
ahojnnes pushed a commit that referenced this pull request Oct 30, 2026
)

Fixes #4787. Supersedes #4895.

## Problem
With `--LomaMatching.use_bf16 1` on CUDA, the bf16 LoMa matcher always
fails to initialize and falls back to fp32:

```
Failed to initialize the bf16 LoMa matcher (This session contains graph nodes that are assigned to the default CPU EP, but fallback to CPU EP has been explicitly disabled by the user.); falling back to fp32
```

The bf16 models are loaded as capability probes, which set
`session.disable_cpu_ep_fallback`. But ONNX Runtime deliberately runs
small shape computations on the CPU, whatever the precision of the
model: its CUDA `Shape`/`Size` kernels write their output to host
memory, and `GetCpuPreferredNodes` then moves their int64 consumers
(`Gather`, `Unsqueeze`, `Concat`, `Cast`, `Reshape`, ...) to the CPU EP.
Tensors in bf16 or fp16 are never moved. The dynamic-shape matcher
graphs contain such nodes, so the probe always rejects them. The fp32
models are not loaded as probes, so they run with the same CPU nodes.

## Fix
Load the bf16 models like any other model, and decide up front whether
to use them:
- The bf16 models are only used with the CUDA execution provider, on
GPUs with compute capability 8.0 (Ampere) or newer. Otherwise, the fp32
models are loaded directly, without first downloading and loading the
bf16 models.
- On older GPUs, creating a bf16 session can succeed while inference
fails, which the fallback on initialization failures would not catch.
- On the CPU, `use_bf16` is now a no-op instead of trying to load the
bf16 models.
- The bf16 models are loaded as regular `ONNXModel`s, with a fallback to
fp32 if initialization throws an `Ort::Exception`. This removes
`ONNXModel`'s `is_capability_probe`, whose only user was LoMa.
- New `CudaDeviceSupportsBf16()` queries the device. It returns false if
the device cannot be queried, and on HIP builds.
- New `SelectONNXCudaDevice()` resolves `gpu_index` to the device that
`ONNXModel` runs on, shared with the bf16 check.
- Updated the option docs (C++, pycolmap, `doc/features.rst`).

This is the same pattern as the fp16 FlexGlue matcher in #4908: reduced
precision only on CUDA, a regular session, and a fallback to fp32 on
initialization failures.

### Comparison with #4895
#4895 keeps the strict probe and instead inspects the EP graph
assignment (ORT >= 1.24) against an allowlist of CPU operators. This PR
avoids:
- the allowlist, which is brittle: which nodes ORT moves to the CPU
depends on data flow, not on the operator type. For example, FlexGlue's
matchers run `Reshape` on the CPU, which the allowlist rejects. Any
rejected node silently falls back to fp32 again.
- the `ORT_API_VERSION >= 24` gating; the fix also works with older ORT.
- downloading and loading the bf16 models on GPUs without bf16 support
before rejecting them.
- ~80 lines of hand-encoded ONNX test models.

Removing the strict probe barely weakens the check: the bf16 tensors
themselves are never moved to the CPU, and the CPU EP has almost no bf16
compute kernels, so a bf16 op without a CUDA kernel still fails at
initialization and falls back to fp32.

## Tests
- `util/cuda_test`: `CudaDeviceSupportsBf16` returns false for invalid
devices and, with a GPU, leaves no CUDA error.
- `util/onnx_test`: `SelectONNXCudaDevice`, and an invalid model throws
the `Ort::Exception` that the LoMa fallback catches.
- `feature/loma_test`, `util/onnx_test`, `util/cuda_test` (also with
`CUDA_VISIBLE_DEVICES=""`) pass locally (CUDA + ONNX build).
- Not yet run end-to-end with the CUDA EP: ORT 1.30's CUDA EP does not
load on my machine (older system CUDA runtime). The bf16 LoMa-B matcher
with CPU shape nodes was validated in #4895 (64/64 and 128/128
self-matches on an RTX 3060).

**Stack:** the FlexGlue stack builds on this PR: #4907, #4921, #4908,
#4915. #4908 selects its fp16 matcher the same way, with a new
`CudaDeviceHasTensorCores()` (compute capability ≥ 7.0) next to
`CudaDeviceSupportsBf16()`. #4915 uses `SelectONNXCudaDevice()` for the
GPU feature cache.
@ahojnnes
ahojnnes force-pushed the sarlinpe/max-num-features branch from 51a39d9 to 4ce2d6d Compare October 30, 2026 08:37
@ahojnnes
ahojnnes enabled stacked auto-merge (squash) October 30, 2026 11:00
@ahojnnes
ahojnnes merged commit d02e670 into main Oct 30, 2026
16 checks passed
@ahojnnes
ahojnnes deleted the sarlinpe/max-num-features branch October 30, 2026 12:09
ahojnnes pushed a commit that referenced this pull request Oct 30, 2026
…4921)

Stacked on #4907. This PR comes before #4908 (FlexGlue matcher), which
reads image files and loads large models while matching, and therefore
needs errors to propagate.

## Problem
An exception thrown in a feature matching worker escapes its `Run()`,
and `Thread::RunFunc` does not catch it, so the process calls
`std::terminate`. For example:
- an unreadable or missing image (FlexGlue, #4908);
- a matcher model that fails to download or load;
- unexpected features, e.g. descriptors of the wrong dimension;
- a corrupt database, e.g. a keypoint blob in an unsupported format,
read by the verifier.

`std::terminate` stops `colmap exhaustive_matcher`, but it also kills
the Python interpreter in pycolmap and the whole GUI, with no exception
and no clean shutdown.

## Changes
- **`FeatureMatcherWorker` and the verifier worker** catch the exception
of a pair and store it in the new `FeatureMatcherData::error`.
- Each worker's per-pair body, `FeatureMatcherWorker::MatchPair()` and
`VerifierWorker::Verify()`, runs entirely inside the `try`. A throw
therefore never ends a worker thread, so the controller never waits
forever for that pair's output.
  - Jobs with an error pass through the remaining workers untouched.
- **`FeatureMatcherController::Match()`** rethrows the first error.
- It first drops the pairs queued for matching. `JobQueue::Clear()` now
returns how many jobs it cleared and wakes waiters.
- Jobs queued for verification are kept, since some of them carry
existing matches that were already deleted from the database.
- It then waits for the pairs in flight and writes their results. The
run therefore stops quickly, and the controller stays usable afterwards.
- **`GeometricVerifierController::Verify()`** rethrows the first error
after processing all pairs, whose matches stay in the database.
- **`FeatureMatcherController::Setup()`** rethrows the error of a worker
that failed to set up, e.g., because its matcher model cannot be loaded.
- The worker has finished, so its `Wait()` returns at once and rethrows.
  - The destructor's later `Wait()` then no longer throws.
- **`Thread`** captures exceptions of `Run()`, logs them (`Thread
failed: …`), and `Wait()` rethrows them, once, in the waiting thread.
- This is needed because the controllers that call
`FeatureMatcherController` are themselves threads, run with
`Start()`/`Wait()` by the CLI and with `PyWait` by pycolmap.
- A thread that fails before signaling its setup unblocks
`CheckValidSetup()` instead of blocking it forever.
- **Result:**
  - pycolmap raises a Python exception.
  - The GUI (`ThreadControlWidget`) shows the error in a dialog.
  - The CLI stops with the error, as before.

**Behavior change:** a thread whose owner never calls `Wait()` now logs
the exception instead of terminating the process. The feature matching
controllers wait on their failed workers in `Setup()`, so their
destructors do not rethrow.

## Tests
- `FeatureMatcherController.MatchPropagatesMatcherErrors`:
- SIFT descriptors of the wrong dimension make the matcher throw for
every pair with one image.
- The error message comes out of `Match()`, and no matches are written
for those pairs.
  - The controller then still matches the other pairs.
- `FeatureMatcherController.SetupPropagatesMatcherCreationErrors`: a
LightGlue model path that does not exist makes `Setup()` throw, and the
controller destructs cleanly.
- `GeometricVerifierController.VerifyPropagatesErrors`: keypoints in an
unsupported format make the verifier throw. `Verify()` rethrows instead
of hanging, and the other pairs are verified.
- `Thread.WaitRethrowsException`.
- **Mutation check:** without the `Setup()` fix the creation test
terminates, and without the verifier `try` the verifier test hangs.
- `util/threading_test`, `controllers/feature_matching_utils_test` and
`controllers/feature_matching_test` pass. With #4908 on top:
- an empty image folder raises `Failed to read image: …` to the caller;
  - nonexistent model paths raise the ONNX Runtime error to the caller;
  - neither calls `std::terminate`.

This addresses review comments on the FlexGlue integration.
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 3, 2026
## Summary
- Integrate all 12 upstream commits after the shared base 9371f7b, including LoMa BF16 loading, matching-worker error propagation, keypoint shape scaling, and project-file CLI overrides.
- Preserve the fork's CUDA blocking-sync helper alongside upstream's BF16 device check and register the shared CUDA test once.

## Validation
- Resolved all merge conflicts; no conflict markers or unmerged paths remain; git diff --check passes.
- Incremental Linux CUDA build linked six focused test executables. The prepared custom Ceres closure required its existing Abseil/CUDA library directories on the linker's search path.
- CTest passed controllers/base_option_manager_test, feature_matching_utils_test, incremental_pipeline_test, option_manager_test, feature/loma_test, and util/cuda_test (six programs, one run each).
- No full suite, Python binding, end-to-end, or performance claim.

## Upstream commits

- `ea470c0c4f4a40a3bdfa33d01eb38375c5df64de` Run Windows and remaining pycolmap CI only with the full-ci label (colmap#4904)
- `c0339e95482c404b51f38ff9b86866de495984f0` Fix MSVC warnings C4805, C4996, and C4756 (colmap#4910)
- `f00d6063bb0b120cf10ee190f7f06f4a34edf432` Expose Ceres solver thresholds in incremental mapping (colmap#4892)
- `978a4467f36bbfc9ae18780f2dca253000b0d008` Keep division-camera derivatives finite on the optical axis (colmap#4888)
- `98f31541f483788ab550f5c4a4e0804e8674b9e0` Fix leak and check covariance in CovarianceWeightedCostFunction (colmap#4911)
- `a467519441171054c7aaa17dffe86fe06431f05c` Avoid stack overflow when compressing union-find chains (colmap#4906)
- `404b83db2c2cc04892e14a578ddfa73e0ac9bfb1` Fix LoMa bf16 fallback on CUDA by loading it like any other model (colmap#4924)
- `d02e6701dede22eaeee89f73784a8c79f6f289ac` Add max_num_features option to automatic_reconstructor and benchmark (colmap#4907)
- `82c3b5db78d96a7e93ca7b8b2af0634e28aa33f4` Propagate errors of feature matching workers instead of terminating (colmap#4921)
- `4afe8d1156a71a2ff33974895f221c8eb3740cb1` Rescale keypoint shapes in image coordinates (colmap#4926)
- `ce2b5ab9df3801c2b75edbba338c04ad3c8cc6af` Fix vocabulary tree builder descriptor dimensions (colmap#4923)
- `f956ce629c21a88d75a7f7b3107298d1faf3dcf3` Fix command-line overrides for project files (colmap#4902)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Run Windows and remaining pycolmap CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants