Skip to content

Fix default prior_position_loss_scale to sqrt(chi2_95_3dof) - #4936

Merged
sarlinpe merged 1 commit into
mainfrom
sarlinpe/ba-prior-loss-scale
Nov 3, 2026
Merged

sarlinpe merged 1 commit into
mainfrom
sarlinpe/ba-prior-loss-scale

Conversation

@sarlinpe

@sarlinpe sarlinpe commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Problem

IncrementalPipelineOptions::prior_position_loss_scale and IncrementalMapper::Options::prior_position_loss_scale default to 7.815 ($\chi^2_{0.95, 3}$), drifting from CeresPosePriorBundleAdjustmentOptions::prior_position_loss_scale (std::sqrt(kChiSquare95ThreeDof) $\approx 2.795$) and overriding it in IncrementalMapper::AdjustGlobalBundle. Because Ceres loss functions (CauchyLoss(a), HuberLoss(a), SoftLOneLoss(a)) apply a to the whitened residual norm ($\rho(s)$ at $s = |r|^2$ with scale $a^2$), passing $a = 7.815$ moves the robust transition to $\approx 7.8\sigma$ instead of the intended 95% 3-DOF threshold $\sqrt{\chi^2_{0.95, 3}} \approx 2.795$.

Fix

  • Align the default prior_position_loss_scale in IncrementalPipelineOptions and IncrementalMapper::Options with CeresPosePriorBundleAdjustmentOptions (std::sqrt(kChiSquare95ThreeDof)).
  • Update the comments and pycolmap docstrings (including the stale CeresPosePriorBundleAdjustmentOptions docstring) to match 2.795.

Tests

  • IncrementalPipelineOptions.DefaultPriorPositionLossScale (src/colmap/controllers/incremental_pipeline_test.cc) verifies that the pipeline, mapper, and Ceres pose-prior BA defaults match std::sqrt(kChiSquare95ThreeDof).

Stack created with GitHub Stacks CLI • Give Feedback 💬

@sarlinpe
sarlinpe added this pull request to stack #4941 October 31, 2026 23:13
@ahojnnes

ahojnnes commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Nice, good find, thank you!

@sarlinpe
sarlinpe force-pushed the sarlinpe/ba-prior-loss-scale branch from ba619d6 to 28ea83b Compare November 1, 2026 23:06
@sarlinpe
sarlinpe removed this pull request from stack #4941 November 1, 2026 23:10
@sarlinpe
sarlinpe added this pull request to stack #4948 November 1, 2026 23:10
@sarlinpe
sarlinpe marked this pull request as ready for review November 2, 2026 23:08
@sarlinpe sarlinpe added the full-ci Run Windows and remaining pycolmap CI label Nov 2, 2026
@sarlinpe
sarlinpe requested a review from ahojnnes November 2, 2026 23:08
@sarlinpe
sarlinpe force-pushed the sarlinpe/ba-prior-loss-scale branch from 28ea83b to a942aa0 Compare November 3, 2026 00:30
@sarlinpe
sarlinpe merged commit 78cc729 into main Nov 3, 2026
16 checks passed
@sarlinpe
sarlinpe deleted the sarlinpe/ba-prior-loss-scale branch November 3, 2026 13:48
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 3, 2026
Summary

Bring official main through78cc72961d79ef6785259e3e8336e5a467dd8ce1,
including fourteen upstream commits since the fork's prior merge base.
This last upstream delta aligns pipeline and mapper prior loss scales with
Ceres at sqrt(kChiSquare95ThreeDof). Explicit overrides are preserved and
robust prior loss stays disabled by default. Original authorship is retained.

Validation

Fresh independent review found no merge-interference/numerical-owner blocker.
A native probe instantiated the actual pipeline, mapper and Ceres pose-prior
option headers; all three defaults equal2.7954834829151074. Existing Ninja
rules refreshed the seven selected consumer targets: option_manager.cc was
rebuilt and six unchanged objects were retained through identity/dependency
checks. No full incremental-pipeline test target or reconstruction was run.

The prior integration gate configured the real project with COLMAP CUDA/ONNX
off and CUDA+ONNX on, compiled seven matching/ONNX/controller objects and
passed six CUDA utility tests on RTX3080Ti/CUDA13.2/GCC12.4. Its unchanged
CUDA binary/objects/XML and loaded libraries were carried by exact identity,
not rerun for this unrelated header/default delta. The imported Ceres package
has CUDA dependencies even in the COLMAP-CUDA-off configuration.

This is a focused incremental qualification; no neural inference, whole-product,
Windows merged-source, HIP, reconstruction-quality or performance claim.

AI-assisted integration. Published history and separate private candidates
are preserved.

## Upstream commits

- `78cc72961d79ef6785259e3e8336e5a467dd8ce1` Fix default prior_position_loss_scale to sqrt(chi2_95_3dof) (colmap#4936)
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 4, 2026
Summary

Bring official main through78cc72961d79ef6785259e3e8336e5a467dd8ce1,
including fourteen upstream commits since the fork's prior merge base.
This last upstream delta aligns pipeline and mapper prior loss scales with
Ceres at sqrt(kChiSquare95ThreeDof). Explicit overrides are preserved and
robust prior loss stays disabled by default. Original authorship is retained.

Validation

Fresh independent review found no merge-interference/numerical-owner blocker.
A native probe instantiated the actual pipeline, mapper and Ceres pose-prior
option headers; all three defaults equal2.7954834829151074. Existing Ninja
rules refreshed the seven selected consumer targets: option_manager.cc was
rebuilt and six unchanged objects were retained through identity/dependency
checks. No full incremental-pipeline test target or reconstruction was run.

The prior integration gate configured the real project with COLMAP CUDA/ONNX
off and CUDA+ONNX on, compiled seven matching/ONNX/controller objects and
passed six CUDA utility tests on RTX3080Ti/CUDA13.2/GCC12.4. Its unchanged
CUDA binary/objects/XML and loaded libraries were carried by exact identity,
not rerun for this unrelated header/default delta. The imported Ceres package
has CUDA dependencies even in the COLMAP-CUDA-off configuration.

This is a focused incremental qualification; no neural inference, whole-product,
Windows merged-source, HIP, reconstruction-quality or performance claim.

AI-assisted integration. Published history and separate private candidates
are preserved.

## Upstream commits

- `78cc72961d79ef6785259e3e8336e5a467dd8ce1` Fix default prior_position_loss_scale to sqrt(chi2_95_3dof) (colmap#4936)
fringehush-gif added a commit to fringehush-gif/colmap that referenced this pull request Nov 7, 2026
Refresh the experimental alternative with fork main while retaining its previous head as a parent. The resulting tree is exactly main plus the isolated shared-current-cost-cache deletion: one CUDA file, +1/-25.

## Change

Read the existing cost map directly during the ordered sampled-cost fold. Preserve current plane propagation, source packing, scheduling and fallback mathematics. The cache originates in custom a37c1c3, MVS: Reuse sampled current costs (GPU perf). The earlier alternative is retained through 8481f89; its original experiment was 8b1412f.

Historical Fountain11 photo+geometry at 3072 recorded 76.85 -> 78.82 seconds, one observation per arm, on the older revision. Retain this inconclusive/adverse observation with its earlier revision; current performance has not been measured. No performance experiment was reopened.

## Validation

- Exact candidate CUDA owner compiled with CUDA 13.2 for compute_86/sm_86. In a bounded retained-support context on RTX 3060, both matched targets passed all 13 selected cases: propagation, ordered source folding, checkpoint-HMM and paired-sweep preservation.
- Four complete inputs and four outputs matched across executables: 8,492,128 bytes per arm. Maps were finite with positive depth and checked ordered graph IDs. Complete witnesses use checkpoint_hmm=false; separate checkpoint tests cover both settings without full cross-executable true-mode witnesses.
- Direct owners, link maps and actual loaded providers were checked; managed jobs ended and released their bookings. Later host resize/warp changes have separate owner qualification. This is not a whole-branch build, reconstruction-quality, portability or current-performance result.

## Upstream commits carried forward

- e52aa8e Move pycolmap macOS CI from macos-14 to macos-15 runner (colmap#4933)
- 8007d1a Preserve float weight ordering in maximum spanning trees (colmap#4928)
- dd84102 Bump pypa/gh-action-pypi-publish from 1.14.2 to 1.14.3 in /.github/workflows (colmap#4942)
- 721e185 Add FlexGlue feature matcher (colmap#4908)
- d0a411f FlexGlue: cache the dense features on the GPU (colmap#4915)
- 105817b Hoist camera model dispatch per row in image warps (colmap#4935)
- 27ffad4 Make benchmark translation-angle errors invariant to reconstruction scale (colmap#4932)
- e8f3522 Resolve benchmark paths before changing subprocess directories (colmap#4931)
- f664d5d Clamp visibility coordinates before unsigned conversion (colmap#4943)
- 138119b Keep thin bitmap thumbnails nonempty (colmap#4927)
- b16ed5f Count rig frame covisibility once per 3D point (colmap#4893)
- 93dbbcb Add colmap-review agent skill for code review (colmap#4934)
- 2a6f695 Fix sift_test build on MSVC without the GUI (colmap#4955)
- 78cc729 Fix default prior_position_loss_scale to sqrt(chi2_95_3dof) (colmap#4936)
- 8653c6b Estimate two-view relative rotation covariance (colmap#4950)
- e1317dd FlexGlue: match keypoints in the resized descriptor input frame (colmap#4949)
- 3db6eb0 Extract ConvertPosePriorsToENU and skip pose priors without position (colmap#4937)
- 77cbe48 Store and estimate rotation covariances on PoseGraph (colmap#4951)
- 30a1a81 Support covariance whitening in CeresRotationAverager (colmap#4952)
- e6644e7 Fix -Wunused-result warning in ThreadPool::AddTask (colmap#4958)
- 5b6da88 Clear descriptor-index results for empty queries (colmap#4960)
- d1d92d9 Add COVARIANCE reweighting for Ceres rotation averaging (colmap#4953)
- 761d90d Enable covariance-weighted rotation averaging in global pipelines (colmap#4823)
- fdee2e1 Reuse the pose graph of GlobalPipeline in the component mappers (colmap#4912)

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