Skip to content

fix: correct MAGSAC++ IRLS gamma weight (B2 + B3) in sigmaConsensusPlusPlus - #49

Draft
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/irls-gamma-b2b3
Draft

fix: correct MAGSAC++ IRLS gamma weight (B2 + B3) in sigmaConsensusPlusPlus#49
f-dy wants to merge 1 commit into
danini:masterfrom
f-dy:fix/irls-gamma-b2b3

Conversation

@f-dy

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

Copy link
Copy Markdown

fix: correct MAGSAC++ IRLS gamma weight (B2 + B3) in sigmaConsensusPlusPlus

Problem

The IRLS refinement weight in MAGSAC::sigmaConsensusPlusPlus (magsac.h)
evaluates the gamma at the wrong argument — the same bug class as in
graph-cut-ransac's getScore, and inconsistent with this file's own
getModelQualityPlusPlus (which is correct):

  • B3 — wrong noise scale. The weight uses sigma_max = current_maximum_sigma
    (= maximum_threshold). The MAGSAC++ paper, and getModelQualityPlusPlus, use
    sigma_max = threshold / k (k = √χ²₀.₉₉(dof)).
  • B2 — table/precision mismatch. The index is computed with the 1e4
    precision against the coarse stored_gamma_values table (step 1e-3),
    reading Γ(1.5, 10·t) — the gamma at 10× the intended argument.

B2 and B3 partially cancel (10/k² ≈ 0.75 for DoF=4), which is why results are
still reasonable — but the IRLS weight is not the paper's, and disagrees with the
marginal scoring in the same class.

Fix

Mirror the already-correct getModelQualityPlusPlus:

// B3: weight noise scale = threshold/k; collection cutoff stays at maximum_threshold
const double weight_sigma_max = current_maximum_sigma * threshold_to_sigma_multiplier;
const double squared_sigma_max_2 = weight_sigma_max * weight_sigma_max * 2.0;
const double one_over_sigma      = C_times_two_ad_dof / weight_sigma_max;
...
// B2: index the fine table (step 1e-4) matching the precision constant
size_t x = round(precision_of_stored_incomplete_gammas * squared_residual / squared_sigma_max_2);
if (stored_incomplete_gamma_number < x) x = stored_incomplete_gamma_number;
weight = one_over_sigma * (stored_complete_gamma_values[x] - gamma_k);

The inlier-collection cutoff (current_maximum_sigma > residual) is unchanged
— the paper truncates the weight at r ≤ k·σ_max = threshold. getModelQualityPlusPlus
is untouched (already correct).

⚠️ Validation / scope

This changes results for the existing (DoF=4) estimators (it removes the
10/k² distortion in the IRLS refinement). Validate on Adelaide / EVD / homogr
(median error and failure rate, before/after) before merging.

Companion graph-cut-ransac PR fixes the identical bug in the duplicated
getScore weight: danini/graph-cut-ransac#53 — released
together. Independent of the per-estimator-DoF PR #45, which is adapted onto this
corrected base separately.

…usPlus

The IRLS refinement weight in sigmaConsensusPlusPlus evaluated the gamma at the
wrong argument, identically to the bug fixed in graph-cut-ransac's getScore:

- B3: the weight used sigma_max = current_maximum_sigma (= maximum_threshold),
  whereas the paper / getModelQualityPlusPlus use sigma_max = threshold / k. Now
  the weight's argument and prefactor use maximum_threshold/k; the inlier
  collection cutoff (current_maximum_sigma) is unchanged.
- B2: the index used precision_of_stored_incomplete_gammas-style precision (1e4)
  against the coarse stored_gamma_values table (step 1e-3) -> gamma at 10x the
  argument. Now indexes the fine stored_complete_gamma_values table.

This makes the IRLS weight match getModelQualityPlusPlus (already correct) and
the MAGSAC++ paper. Changes results for the existing (DoF=4) estimators; validate
on Adelaide/EVD/homogr before submission. Independent of danini#45 (per-estimator DoF),
adapted onto this corrected base separately. Depends on the companion
graph-cut-ransac PR for the duplicated getScore weight.
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