Repository navigation
Add max_num_features option to automatic_reconstructor and benchmark - #4907
Merged
Merged
Conversation
sarlinpe
enabled stacked auto-merge (squash)
October 28, 2026 20:34
sarlinpe
force-pushed
the
sarlinpe/max-num-features
branch
from
October 29, 2026 05:43
f44de67 to
ab6adb4
Compare
stacked auto-merge was automatically disabled
October 29, 2026 05:57
Pull request removed from stack
sarlinpe
force-pushed
the
sarlinpe/max-num-features
branch
from
October 29, 2026 23:27
ab6adb4 to
51a39d9
Compare
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
force-pushed
the
sarlinpe/max-num-features
branch
from
October 30, 2026 08:37
51a39d9 to
4ce2d6d
Compare
ahojnnes
approved these changes
Oct 30, 2026
ahojnnes
enabled stacked auto-merge (squash)
October 30, 2026 11:00
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
max_num_featuresoption toautomatic_reconstructorand 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).max_num_featuresof the SIFT, ALIKED and LoMa extraction options.colmap automatic_reconstructor --max_num_features N.benchmark/reconstruction/evaluate.py --max_num_features Nforwards the option to everyautomatic_reconstructorcall (evaluation/utils.py).Validation
--feature loma --max_num_features {2048, 4096, 8192}, all 898 images of ETH3D DSLR have exactly 2048 / 4096 / 8192 keypoints.controllers/tests pass.Part of the FlexGlue stack: #4924, then this PR, #4921, #4908 and #4915. #4908 uses it for its evaluation.