fix: derive MAGSAC reference threshold from sigma_max (existing estimators) - #47
Draft
f-dy wants to merge 1 commit into
Draft
fix: derive MAGSAC reference threshold from sigma_max (existing estimators)#47f-dy wants to merge 1 commit into
f-dy wants to merge 1 commit into
Conversation
…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.
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.
fix: derive the MAGSAC reference threshold from sigma_max (existing estimators)
Problem
The pybind
find*_functions insrc/pymagsac/src/magsac_python.cppleave the MAGSACreference threshold (
interrupting_threshold,magsac.h) at its constructor defaultof 1.0. It counts "confident" inliers (
residual < interrupting_threshold), whichdrives the adaptive iteration count:
1.0is not a principled value: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.1.0is in data units, soidentical 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 rescalingthe arbitrary
1.0by the coordinate normalizer (still rooted in1.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 inlierthreshold
τ = k·σ_max— scale-invariant and consistent with the MAGSAC++ weight/losscutoff. Using
getSigmaQuantile()(rather than a hardcodedk) keeps it correct beforeand 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
masteron 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.