Skip to content

fix: derive MAGSAC reference threshold from sigma_max (unit-independent) - #46

Closed
f-dy wants to merge 2 commits into
danini:masterfrom
f-dy:fix/reference-threshold-units
Closed

fix: derive MAGSAC reference threshold from sigma_max (unit-independent)#46
f-dy wants to merge 2 commits into
danini:masterfrom
f-dy:fix/reference-threshold-units

Conversation

@f-dy

@f-dy f-dy commented May 31, 2026

Copy link
Copy Markdown

fix: derive the MAGSAC reference threshold from sigma_max (unit-independent)

Problem

The pybind find*_ functions in src/pymagsac/src/magsac_python.cpp leave the
MAGSAC reference threshold (interrupting_threshold, magsac.h:55) at its
constructor default of 1.0. This threshold counts "confident" inliers
(residual < interrupting_threshold), which drives the adaptive iteration count:

last_iteration_number = log(1−conf) / log(1 − (inlier_number / N)^sample_size);

1.0 is not a principled value:

  • Pixel estimators: SIFT at coarse scales (and ASIFT, dense matches) has inlier
    localization error of several pixels; inlier thresholds are commonly set to 3–5 px.
    With a 1.0-px reference, genuine inliers with 1–3 px residuals are excluded from
    the confident count, under-estimating the inlier ratio.
  • Metric estimators (rigid transformation, plane, line): 1.0 is in data units,
    so identical geometry in mm vs m vs inches yields different iteration budgets.

The reference threshold affects the RANSAC search budget — the number of samples
drawn — and therefore, on hard problems, which model is found. It does not change
the per-hypothesis weight/loss math.

Previously only findEssentialMatrix_ touched it, and merely rescaled the arbitrary
1.0 default by the coordinate normalizer (still rooted in 1.0).

Fix

Derive the reference threshold from the noise scale, uniformly across all estimators:

magsac->setReferenceThreshold(EstimatorType::getSigmaQuantile() * sigma_max);

k = getSigmaQuantile() = sqrt(chi2_0.99(DoF)) makes this the statistical inlier
threshold τ = k·σ_max — scale-invariant and consistent with the MAGSAC++ weight/loss
cutoff (the weight is exactly 0 beyond k·σ_max). The normalized essential estimator
uses k · normalized_sigma_max.

Scope & relationship to other PRs

f-dy added 2 commits May 31, 2026 19:00
…, essential variants)

Add Python bindings for models available in graph-cut-ransac:
- findPlane3D (3D plane fitting)
- findPnP (camera pose via P3P)
- findRadialHomography (homography + radial distortion)
- findHomographyAffine (homography from affine correspondences)
- findFundamentalMatrixAffine (fundamental from affine/SIFT)
- findEssentialMatrixPlanar (essential for planar motion)
- findEssentialMatrixGravity (essential with gravity prior)

Fix degrees of freedom for all MAGSAC estimators:
- Fundamental/Essential: DoF=4 (Sampson distance, unchanged)
- Homography/PnP/Radial/Affine: DoF=2 (2D transfer error)
- Rigid transformation: DoF=3 (3D Euclidean distance)
- Line2D/Plane3D: DoF=1 (scalar signed distance)

The DoF fix uses:
- Precomputed E₁(x) table for DoF=1 (gamma_values_dof1.cpp)
- Closed-form erfc(√x) for DoF=2
- Closed-form exp(-x) for DoF=3
- Existing stored_gamma_values table for DoF=4

CMake detects if gamma_values_dof1.cpp is available from the
graph-cut-ransac submodule; falls back to local copy if not.
The pybind find*_ functions left the MAGSAC reference threshold
(interrupting_threshold) at its constructor default of 1.0 — a pixel-unit
assumption. It counts 'confident' inliers (residual < interrupting_threshold),
which drives the adaptive iteration count
  N = log(1-conf) / log(1 - (inliers/N)^sample_size).
A fixed 1.0 is not principled even for pixels (SIFT at coarse scales has
inlier localization error of several pixels), and is unit-dependent for metric
estimators (rigid transformation, plane, line). It affects the RANSAC search
budget — and thus, on hard problems, which model is found — not the
per-hypothesis weight/loss math.

Derive it from the data instead: setReferenceThreshold(k * sigma_max) with
k = estimator.getSigmaQuantile() = sqrt(chi2_0.99(DoF)) (the statistical inlier
threshold tau = k*sigma_max). Applied to all estimators, including the
normalized essential estimator (which previously rescaled the arbitrary 1.0
default by the coordinate normalizer).

NOTE: plane/line already received this fix in danini#45. This change
covers the remaining (pre-existing and pixel-domain) estimators and should be
validated on the paper's datasets (Adelaide/EVD/homogr) for failure-rate
regressions before merging — see benchmarks/.
@f-dy

f-dy commented May 31, 2026

Copy link
Copy Markdown
Author

Closing: this draft was branched off #45 but targeted master, so the diff redundantly includes all of #45 (~16k lines, the gamma table + DoF constants). The actual change is 60 lines in magsac_python.cpp (setReferenceThreshold = k*sigma_max). Will reopen against master once #45 merges, for a clean diff. Branch fix/reference-threshold-units is preserved.

@f-dy f-dy closed this May 31, 2026
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.

1 participant