Skip to content

fix: correct MAGSAC++ IRLS gamma weight (B2 + B3) in getScore#53

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 getScore#53
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 getScore

Problem

MAGSACScoringFunction::getScore (scoring_function.h) computes the per-point
MAGSAC++ weight at the wrong gamma argument, due to two compounding bugs:

  • B3 — wrong noise scale. It uses sigma_max = increasedThreshold (the
    inlier threshold itself). The MAGSAC++ paper — and this repo's own
    getModelQualityPlusPlus — use sigma_max = threshold / k, where
    k = √χ²₀.₉₉(dof). The function even defines threshold_to_sigma_multiplier = 1/k
    but never applies it.
  • B2 — table/precision mismatch. The index is
    round(precision_of_stored_gammas · …) with precision_of_stored_gammas = 1e4,
    but stored_gamma_values is spaced at 1e-3 ([0, 36], 36 001 entries). So
    the lookup reads Γ(1.5, 10·t) — the gamma at 10× the intended argument.

B2 and B3 partially cancel (net factor 10/k² ≈ 0.75 for DoF=4), which is why
MAGSAC++ still works well in practice — but the weight is not the paper's.

Fix

// B3: weight noise scale = threshold/k; collection cutoff stays at threshold
const double sigma_max = increasedThreshold * threshold_to_sigma_multiplier;
const double squared_sigma_max_2 = sigma_max * sigma_max * 2.0;
const double one_over_sigma      = C_times_two_ad_dof / sigma_max;
...
// B2: index the fine table (step 1e-4) whose spacing matches the 1e4 precision
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);
  • sigma_max = threshold/k drives both the gamma argument and the prefactor.
  • The inlier-collection cutoff (residual > increasedThreshold) is unchanged
    — the paper truncates the weight at r ≤ k·σ_max = threshold.
  • The lookup moves to the fine stored_complete_gamma_values table, whose
    1e-4 spacing matches precision_of_stored_incomplete_gammas.

After this, getScore's weight matches getModelQualityPlusPlus and the paper.

Tables: generated + trimmed to the useful interval

gamma_values.cpp is now produced by scripts/generate_gamma_values.py
(scipy.special), which emits the C++ with a header comment documenting how it was
generated and why its length. The two live DoF=4 tables
(stored_complete_gamma_values = Γ(1.5,·), stored_lower_incomplete_gamma_values
= γ(2.5,·)) are trimmed to the useful interval [0, k²/2] — 66 385 entries,
down from [0, 10] = 100 001 — because the weight is truncated at the inlier
cutoff r ≤ k·σ_max, so the argument never exceeds k²/2. The dead coarse
table
(stored_gamma_values / precision_of_stored_gammas /
stored_gamma_number) is removed (it had no remaining consumer once getScore
moved to the fine table — verified across the whole gcransac repo).

The table is sized for the precise k = √χ²₀.₉₉(4) (the largest plausible
cutoff), so it works unchanged whether the consumer uses the imprecise k literal
or a precise one; lookups clamp to stored_incomplete_gamma_number.

Note on k precision (deferred to #50)

This PR keeps the existing imprecise k literal (k(DoF=4) = 3.64, with its
matching gamma_k) to stay focused on the B2/B3 weight bug. Precise per-DoF
k = √χ²₀.₉₉(DoF) is introduced by the per-estimator-DoF PR (#50). Because the
table is sized for the precise k, #50 can switch to precise k without
regenerating or resizing the table.

⚠️ Validation / scope

This changes results for the existing (DoF=4) estimators (it removes the
10/k² distortion). It must be validated on the paper's datasets
(Adelaide / EVD / homogr: median error and failure rate, before/after)
before merging.

Companion magsac PR fixes the identical bug in magsac.h::sigmaConsensusPlusPlus:
danini/magsac#49 — the two are released together. Independent of the per-estimator-DoF PR (#50),
which is adapted onto this corrected base separately. The now-unused coarse
stored_gamma_values / precision_of_stored_gammas can be retired once #50's
adaptation also stops using them.

…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