Skip to content

fix: derive MAGSAC reference threshold from sigma_max (existing estimators) - #47

Draft
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/reference-threshold-units
Draft

fix: derive MAGSAC reference threshold from sigma_max (existing estimators)#47
f-dy wants to merge 1 commit 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 (existing estimators)

Problem

The pybind find*_ functions in src/pymagsac/src/magsac_python.cpp leave the MAGSAC
reference threshold (interrupting_threshold, magsac.h) at its constructor default
of 1.0. It 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 (homography, fundamental, essential): SIFT at coarse scales (and
    ASIFT, dense matches) has inlier localization error of several pixels; inlier
    thresholds are commonly 3–5 px. A 1.0-px reference under-counts genuine inliers.
  • Metric estimators (rigid transformation, line): 1.0 is in data units, so
    identical geometry in mm vs m vs inches yields different iteration budgets.

This 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, merely rescaling
the arbitrary 1.0 by the coordinate normalizer (still rooted in 1.0).

Fix

Derive the reference threshold from the noise scale, uniformly:

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. Using getSigmaQuantile() (rather than a hardcoded k) keeps it correct before
and after any per-estimator DoF change. The normalized essential estimator uses
k · normalized_sigma_max.

Scope

Covers the pre-existing estimators on master: rigid transformation, fundamental,
essential, line, homography. Independent of other PRs — branched from and mergeable
into master on its own (22-line diff, one file).

The estimators added in #45 (plane, PnP, radial homography, affine,
planar/gravity essential) set this from the start in that PR, so they are not touched
here.

Note on validation

This changes the iteration budget of estimators that existing users rely on. It is
low-severity (search budget / runtime, not the per-hypothesis scoring), but should be
validated on the paper's datasets (Adelaide / EVD / homogr) for failure-rate / accuracy
regressions before merging.

…g estimators

The pybind find*_ functions left the MAGSAC reference threshold
(interrupting_threshold, magsac.h) at its constructor default of 1.0 — a
pixel-unit assumption. It counts 'confident' inliers
(residual < interrupting_threshold), driving the adaptive iteration count
  N = log(1-conf) / log(1 - (inliers/N)^sample_size).
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 data
(rigid transformation, line). It affects the RANSAC search budget — and thus,
on hard problems, which model is found — not the per-hypothesis weight/loss.

Derive it from the data: setReferenceThreshold(k * sigma_max) with
k = getSigmaQuantile() = sqrt(chi2_0.99(DoF)). Using getSigmaQuantile() makes
this forward-compatible with the per-estimator DoF fix. The normalized
essential estimator now uses k * normalized_sigma_max instead of rescaling the
arbitrary 1.0 default by the coordinate normalizer.

Covers the pre-existing estimators (rigid, fundamental, essential, line,
homography). The estimators added in danini#45 set this from the start.

Affects only the iteration budget (runtime / hard-case search), not the
per-hypothesis scoring. Should be validated on the paper's datasets
(Adelaide/EVD/homogr) for failure-rate regressions before merging.
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