Repository navigation
Propagate errors of feature matching workers instead of terminating - #4921
Merged
ahojnnes merged 1 commit intoOct 30, 2026
Merged
Conversation
This was referenced Oct 29, 2026
sarlinpe
force-pushed
the
sarlinpe/matching-error-propagation
branch
from
October 29, 2026 07:27
d259051 to
d8354e2
Compare
sarlinpe
force-pushed
the
sarlinpe/matching-error-propagation
branch
from
October 29, 2026 23:27
d8354e2 to
3c0529a
Compare
An exception thrown while matching or verifying a pair (e.g., an unreadable image, unexpected features, or a corrupt database) used to escape the worker thread and terminate the process, which also kills pycolmap sessions and the GUI. - FeatureMatcherWorker and the verifier worker catch the exception of a pair and store it in FeatureMatcherData::error. Jobs with an error pass through the remaining workers untouched. - FeatureMatcherController::Match() drops the pairs queued for matching (JobQueue::Clear() now returns the number of cleared jobs), waits for the pairs in flight, writes their results, and rethrows the first error. Jobs queued for verification are kept, since they may carry existing matches that were already deleted from the database. GeometricVerifierController::Verify() rethrows the first error after processing all pairs. - Thread captures the exceptions of Run() and Wait() rethrows them in the waiting thread, since the controllers themselves run in threads. A thread that fails before signaling its setup unblocks CheckValidSetup(). FeatureMatcherController::Setup() rethrows the error of a worker that failed to set up, e.g., because its matcher model cannot be loaded. pycolmap therefore raises a Python exception, and the GUI shows an error dialog.
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/matching-error-propagation
branch
from
October 30, 2026 08:37
3c0529a to
1ac63f2
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
…4907) ## 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.
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.
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(), andThread::RunFuncdoes not catch it, so the process callsstd::terminate. For example:std::terminatestopscolmap exhaustive_matcher, but it also kills the Python interpreter in pycolmap and the whole GUI, with no exception and no clean shutdown.Changes
FeatureMatcherWorkerand the verifier worker catch the exception of a pair and store it in the newFeatureMatcherData::error.FeatureMatcherWorker::MatchPair()andVerifierWorker::Verify(), runs entirely inside thetry. A throw therefore never ends a worker thread, so the controller never waits forever for that pair's output.FeatureMatcherController::Match()rethrows the first error.JobQueue::Clear()now returns how many jobs it cleared and wakes waiters.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.Wait()returns at once and rethrows.Wait()then no longer throws.Threadcaptures exceptions ofRun(), logs them (Thread failed: …), andWait()rethrows them, once, in the waiting thread.FeatureMatcherControllerare themselves threads, run withStart()/Wait()by the CLI and withPyWaitby pycolmap.CheckValidSetup()instead of blocking it forever.ThreadControlWidget) shows the error in a dialog.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 inSetup(), so their destructors do not rethrow.Tests
FeatureMatcherController.MatchPropagatesMatcherErrors:Match(), and no matches are written for those pairs.FeatureMatcherController.SetupPropagatesMatcherCreationErrors: a LightGlue model path that does not exist makesSetup()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.Setup()fix the creation test terminates, and without the verifiertrythe verifier test hangs.util/threading_test,controllers/feature_matching_utils_testandcontrollers/feature_matching_testpass. With Add FlexGlue feature matcher #4908 on top:Failed to read image: …to the caller;std::terminate.This addresses review comments on the FlexGlue integration.