Skip to content

fix: log_prior_from_value sign-convention bug across Prior subclasses #1266

Description

@Jammy2211

Overview

Prior.log_prior_from_value has a 4-year-old sign-convention bug across multiple Prior subclasses, surfaced while researching a related LogUniformPrior issue (PR #1263 follow-up).

The library's consumer chain expects log_prior_from_value to return the prior log-density log p(x) (negative for low-density regions), since Fitness._call adds it to log_likelihood to produce a log-posterior figure_of_merit consumed by Emcee/Zeus as a maximand. But the existing implementations actually return the cost form -log p(x) (positive for low-density regions) — i.e. the wrong sign — except for UniformPrior which returns 0.0 and is sign-agnostic, and LogUniformPrior which returns numerical garbage (the Jacobian gradient 1/x, neither a log nor a cost).

Evidence

NormalMessage.log_prior_from_value (autofit/messages/normal.py:430)

def log_prior_from_value(self, value: float, xp=np) -> float:
    return (value - self.mean) ** 2.0 / (2 * self.sigma**2.0)

Returns the positive quadratic — a cost (-log p up to a constant), not a density. For a flat likelihood, an Emcee fit would push samples away from mean, which is the opposite of what a Gaussian prior should enforce.

TruncatedNormalMessage.log_prior_from_value (autofit/messages/truncated_normal.py:479)

Mostly cost form but uses xp.where(in_bounds, log_trunc_pdf, -xp.inf) — making the in-bounds region cost-form and the out-of-bounds region density-form. Internally inconsistent.

LogGaussianPrior.log_prior_from_value (autofit/mapper/prior/log_gaussian.py:136)

return self.message.base_message.log_prior_from_value(np.log(value)) - np.log(value)

Inherits the wrong sign from NormalMessage (cost-form quadratic), plus a -log(value) Jacobian. Net: still wrong sign on the quadratic.

UniformPrior.log_prior_from_value (autofit/mapper/prior/uniform.py:160)

Returns 0.0 (NumPy path) — sign-agnostic, accidentally correct.

LogUniformPrior.log_prior_from_value (autofit/mapper/prior/log_uniform.py:113)

return 1.0 / value

Neither density nor cost form — this is literally d(log x)/dx, the Jacobian gradient, not its log. Pure bug.

Consumer Impact

autofit/non_linear/fitness.py:200 in Fitness._call:

log_prior_array = self._xp.array(self.model.log_prior_list_from_vector(...))
figure_of_merit = log_likelihood + self._xp.sum(log_prior_array)
return figure_of_merit
Sampler How figure_of_merit is consumed Bug manifestation
Emcee / Zeus log_prob_fn to maximise via Fitness.call_wrap Biased posterior — Gaussian priors push samples away from mean (positive-quadratic cost added to log-likelihood). Bug is small for narrow priors dominated by likelihood; significant for weak/wide priors.
Dynesty / Nautilus Pure log-likelihood via fom_is_log_likelihood=True; priors flow through prior_transform only Unaffectedlog_prior_from_value is never consulted on the hot path.
LBFGS / BFGS convert_to_chi_squared=True then minimised Accidentally correct — the × -2 chi-squared conversion flips the sign back.
MLE Drawer fom_is_log_likelihood=False, treats figure_of_merit as log-posterior Same bias as Emcee/Zeus.
Graphical EP Routes through Model.log_posterior_from_vector → same call Same bias as Emcee.

Why this hasn't been noticed before

  • Most production lensing/galaxy fits use Nautilus/Dynesty (unaffected).
  • Most MCMC fits use UniformPrior (sign-agnostic 0.0).
  • Tight Gaussian priors on physically-constrained parameters produce posteriors dominated by the likelihood, where the small wrong-sign quadratic acts as a weak nuisance rather than a clear failure mode.
  • The pinning test in test_autofit/mapper/prior/test_prior.py:192-219 was written against the buggy values — it rubber-stamps the bug.

Proposed fix

  1. Decide convention library-wide: return density log p(x) (negative for low-density), matching standard probabilistic-programming conventions and what Fitness._call's additive form expects.

  2. Sign-flip:

    • NormalMessage.log_prior_from_value: return -(value - mean)**2 / (2 * sigma**2) (drop the -log(σ√(2π)) constant, consistent with UniformPrior dropping -log(b-a)).
    • TruncatedNormalMessage.log_prior_from_value: flip the in-bounds branch from cost to density. Out-of-bounds -inf already correct.
    • LogGaussianPrior.log_prior_from_value: flip the quadratic; keep the -log(value) Jacobian.
  3. Replace LogUniformPrior.log_prior_from_value:

    def log_prior_from_value(self, value, xp=np):
        if xp is np:
            return -np.log(value)
        in_bounds = (value >= self.lower_limit) & (value <= self.upper_limit)
        return xp.where(in_bounds, -xp.log(value), -xp.inf)

    Dropping the -log(log(b/a)) constant for consistency with UniformPrior dropping -log(b-a).

  4. Audit Fitness.call_wrap (fitness.py:251) which does log_likelihood -= np.sum(log_prior_list) to recover pure log-likelihood from figure_of_merit — the sign here is consistent with the additive _call line and stays correct after the flip.

  5. Update the pinning test test_autofit/mapper/prior/test_prior.py:192-219 with the corrected values.

  6. Verification:

    • Full pytest test_autofit — surface any test that pins the wrong values.
    • End-to-end Emcee fit on a flat likelihood with GaussianPrior(mean=μ, sigma=σ) — confirm samples cluster around μ post-fix (today they spread away from μ).
    • Run autofit_workspace searches/mcmc.py — confirm it still completes.
    • Run autofit_workspace_test/scripts/graphical/ep.py — EP posterior values will shift; confirm direction is correct (closer to the prior mean for under-constrained parameters).

Release-notes warning

Any cached samples.csv from prior MCMC fits (Emcee/Zeus/MLE Drawer) with non-uniform priors has biased posterior values and should be re-run after this fix lands. Nested-sampler results (Dynesty/Nautilus) are unaffected. LBFGS/BFGS results are unaffected (sign accidentally correct via chi-squared).

Scope

PyAutoFit only. No workspace changes needed, but workspace searches/mcmc.py smoke run is part of the validation gate.

Background

Surfaced during the deep research phase of issue #1262 follow-up work (LogUniformPrior 1/value formula). The original task scope was a one-line LogUniformPrior fix; the audit revealed the broader sign-convention bug across the Gaussian-family priors.

The 1.0 / value formula has been in LogUniformPrior since commit db4016db42 (4 May 2022, "refactored priors into package"). The Gaussian-family cost-form signs predate that.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions