fix: derive MAGSAC reference threshold from sigma_max (unit-independent) - #46
Closed
f-dy wants to merge 2 commits into
Closed
fix: derive MAGSAC reference threshold from sigma_max (unit-independent)#46f-dy wants to merge 2 commits into
f-dy wants to merge 2 commits into
Conversation
…, 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/.
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. |
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 (unit-independent)
Problem
The pybind
find*_functions insrc/pymagsac/src/magsac_python.cppleave theMAGSAC reference threshold (
interrupting_threshold,magsac.h:55) at itsconstructor default of 1.0. This threshold counts "confident" inliers
(
residual < interrupting_threshold), which drives the adaptive iteration count:1.0is not a principled value: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 fromthe confident count, under-estimating the inlier ratio.
1.0is 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 arbitrary1.0default by the coordinate normalizer (still rooted in1.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 inlierthreshold
τ = k·σ_max— scale-invariant and consistent with the MAGSAC++ weight/losscutoff (the weight is exactly 0 beyond
k·σ_max). The normalized essential estimatoruses
k · normalized_sigma_max.Scope & relationship to other PRs
affine, planar/gravity essential) are added by feat: add new model estimators (plane, PnP, radial homography) #45. The plane/line reference-threshold
fix already ships in feat: add new model estimators (plane, PnP, radial homography) #45 (new metric estimators, no existing behavior to regress). This
PR covers the remaining pre-existing and pixel-domain estimators (rigid, fundamental,
homography, PnP, radial, affine, essential variants). Rebase onto
masterafter feat: add new model estimators (plane, PnP, radial homography) #45 mergesfor a clean diff.
changes the iteration budget of estimators that existing users rely on. Validate on the
paper's datasets (Adelaide / EVD / homogr) for failure-rate / accuracy regressions before
merging. A synthetic harness is available in the project's
benchmarks/.it is separated from the bug-fix PRs.