Skip to content

fix: use per-estimator degrees of freedom in MAGSAC scoring#50

Draft
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/magsac-dof
Draft

fix: use per-estimator degrees of freedom in MAGSAC scoring#50
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/magsac-dof

Conversation

@f-dy

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

Copy link
Copy Markdown

⚠️ Do not merge standalone — rescoped. The per-estimator DoF change is
statistically entangled with the B2/B3 IRLS gamma fix through the shared
k=√χ²₀.₉₉(n) (the B2×B3 10/k² cancellation is DoF-dependent), and shipping
DoF alone propagates B2 into the new DoFs. So this DoF work should land
together with the B2/B3 IRLS gamma fix (magsac
danini/magsac#49 + gcransac
#53) as one benchmarked PR. The unrelated
null-sample_ crash guard has been split out to
#52. This branch (fix/magsac-dof)
already carries DoF + B1 and is the basis for that combined PR.

Summary

The MAGSACScoringFunction in scoring_function.h hardcodes _DimensionNumber = 4 for all estimator types. This is only correct for the Sampson distance used by fundamental/essential matrix estimation (where the residual is derived from 4D data (x1,y1,x2,y2) and (r/σ)² ~ χ²(4)).

For other estimators, the degrees of freedom of the squared residual are different:

Estimator Residual type Correct DoF Was using
Fundamental/Essential Sampson distance 4 4 ✓
Homography 2D transfer error dx²+dy² 2 4 ✗
PnP 2D reprojection error du²+dv² 2 4 ✗
Radial homography 2D transfer error 2 4 ✗
Homography (affine) 2D transfer error 2 4 ✗
Rigid transformation 3D Euclidean dx²+dy²+dz² 3 4 ✗
Linear model (plane/line) Scalar signed distance 1 4 ✗

The wrong DoF propagates to:

  • k (the χ² quantile used as sigma-to-threshold multiplier): e.g., √χ²₀.₉₉(2) = 3.03 vs the hardcoded 3.64 = √χ²₀.₉₉(4)
  • gamma_k (the upper incomplete gamma at the boundary)
  • The entire lookup table (precomputed for Γ(1.5, x) = DoF=4 only)

The practical effect is that MAGSAC weights decay with the wrong tail shape — χ²(4) has a fatter tail than χ²(1) or χ²(2), so the scoring is more permissive of outliers than the statistical model warrants.

Fix

  1. Add static constexpr size_t getDegreesOfFreedom() to each estimator, returning the correct value for its residual type.

  2. Add precomputed E₁(x) table (gamma_values_dof1.cpp) for DoF=1, since E₁ has no closed-form expression in standard C++.

  3. Use closed-form expressions for DoF=2 and DoF=3:

    • DoF=2: Γ(0.5, x) = √π · erfc(√x) via std::erfc
    • DoF=3: Γ(1, x) = e⁻ˣ via std::exp
  4. Use existing stored_gamma_values[] table for DoF=4 (unchanged).

  5. Dispatch via if constexpr in the scoring loop, with a static_assert in the else branch to catch unsupported DoF values at compile time.

Backward compatibility

  • For DoF=4 (fundamental/essential), the code uses the exact same stored_gamma_values[] table — results are numerically identical.
  • The gamma_values.cpp file is still included and used for DoF=4.
  • No new dependencies; uses only <cmath> functions (erfc, exp, sqrt).
  • Adding a model with DoF≥5 without providing a gamma implementation triggers a compile-time error.

Verification

chi2_0.99(1) = 6.6349, k = 2.5758  (was using k=3.64)
chi2_0.99(2) = 9.2103, k = 3.0349  (was using k=3.64)
chi2_0.99(3) = 11.345, k = 3.3682  (was using k=3.64)
chi2_0.99(4) = 13.277, k = 3.6437  (unchanged)

Split out: null-pointer crash in solver_linear_model.h

The null-sample_ crash guard that was originally bundled here has been
extracted to its own standalone PR, #52
(it has zero coupling to the gamma/DoF work and can merge independently).

DoF=1 weight singularity (fixed)

The E₁(x) table for DoF=1 has E₁(0)=∞ at index 0. When a small residual rounds to
index 0, the lookup returned ∞ → infinite weight. Fixed by capping table[0] = E₁(step)
(first finite entry) in gamma_values_dof1.cpp.

Known gamma-weight issues (NOT fixed here — flagged for maintainers)

Reviewing MAGSACScoringFunction in scoring_function.h against the MAGSAC++ paper
(Eq. 2.1) revealed two pre-existing inaccuracies, also present in magsac's IRLS:

  • B2 — table-step/precision mismatch: the coarse table stores Γ(a, i/1000) (step 1/1000) but is indexed with precision_of_stored_gammas = 1e4 → the gamma is evaluated at 10× the intended argument.
  • B3 — σ_max definition: the scoring uses σ_max = increasedThreshold directly; the paper uses σ_max = threshold / k.
  • B2 and B3 partially cancel (10/k² ≈ 0.75 for n=4). Fixing B2 alone (precision = 1e3) is harmful — it removes the cancellation (argument becomes 1/k² of correct). Both must be fixed together. Not done here: changes results for all models and needs accuracy benchmarks.

@f-dy f-dy force-pushed the fix/magsac-dof branch 2 times, most recently from 7a0aacc to 0faa649 Compare May 29, 2026 15:23
@f-dy f-dy marked this pull request as ready for review May 29, 2026 15:29
@f-dy f-dy marked this pull request as draft May 29, 2026 15:37
@f-dy f-dy force-pushed the fix/magsac-dof branch 6 times, most recently from 0041613 to d0459ba Compare May 31, 2026 22:57
The MAGSACScoringFunction hardcoded DoF=4 (appropriate for Sampson
distance on 2D-2D correspondences) for all estimator types. This is
incorrect for:
- Linear model (plane/line): scalar distance -> chi^2(1)
- Homography/PnP/radial homography: 2D transfer error -> chi^2(2)
- Rigid transformation: 3D Euclidean distance -> chi^2(3)

Fix:
- Add static constexpr getDegreesOfFreedom() to each estimator
- Add precomputed E1(x) table for DoF=1 (gamma_values_dof1.cpp)
- Use closed-form expressions for DoF=2 (erfc) and DoF=3 (exp)
- Use existing stored_gamma_values table for DoF=4 (unchanged)
- Dispatch via if constexpr in the scoring loop

Backward compatible: DoF=4 path uses the exact same table as before.
@f-dy f-dy force-pushed the fix/magsac-dof branch from d0459ba to 48a8bef Compare May 31, 2026 23:44
f-dy added a commit to f-dy/graph-cut-ransac that referenced this pull request Jun 1, 2026
…oF=4 tables

getScore computed the per-point MAGSAC++ weight at the wrong gamma argument:
- B3: sigma_max was increasedThreshold (the inlier threshold) rather than
  threshold/k. Now sigma_max = increasedThreshold/k drives both the argument and
  the prefactor; the inlier-collection cutoff stays at increasedThreshold = k*sigma_max.
- B2: the index used the 1e4 precision against the coarse stored_gamma_values
  table (step 1e-3), reading the gamma at 10x the argument. Now it indexes the
  fine stored_complete_gamma_values table (step 1e-4) whose spacing matches 1e4.

Tables (gamma_values.cpp) are now generated by scripts/generate_gamma_values.py
(scipy.special) and trimmed to the useful interval [0, k^2/2]: 66385 entries
(was [0,10]=100001). The dead coarse table is removed. The generated C++ documents
how it was produced and why the length; the table is sized for the precise
k = sqrt(chi2_0.99(4)) so it works with either the imprecise k literal kept here
(3.64) or the precise per-DoF k introduced by danini#50.

k stays at the existing imprecise literal in this PR (focused on B2/B3); precise
per-DoF k is danini#50's job. Changes results for existing DoF=4 estimators: validate
on Adelaide/EVD/homogr before submission.
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