Skip to content

fix: LogGaussianPrior declares its own (0, inf) support #1526

Description

@Jammy2211

Overview

LogGaussianPrior's support is (0, inf)log_prior_from_value returns -inf
for value <= 0 — but the prior reports (-inf, inf). Its TransformedMessage
defaults its limits to ±inf and is never passed any, so Prior.__getattr__
delegates a wrong answer. Every other prior answers truthfully. This is follow-up 3
owed by the prior-support Clipper (PyAutoFit#1477), which worked around it with an
isinstance special case rather than fixing it.

The hazard is not cosmetic: a -inf lower bound on a strictly positive parameter
means consumers do not guard 0, and log(0) or a division by it is the failure
that follows. Two consumers are already getting the wrong answer — see "Measured
impact" below.

Plan

  • Declare the real support on LogGaussianPrior itself: lower_limit = 0.0,
    upper_limit = inf, shadowing on the prior exactly as UniformPrior and
    LogUniformPrior already do.
  • Add a general strictness contract to Prior (lower_limit_strict /
    upper_limit_strict, default False; True for LogGaussian's lower bound) so
    consumers can tell an exclusive bound from an inclusive one without a type switch.
  • Derive Prior.limits from lower_limit / upper_limit so the two notions cannot
    disagree by construction — the actual root cause of this bug class.
  • Retire ClipperPriorBox._limits_from_model's isinstance(prior, LogGaussianPrior)
    block, its import, and the docstring paragraphs documenting the workaround.
  • Lock it all with tests, including the general property that a prior's reported
    support matches its actual support, parametrised over every prior family.

Measured impact

Verified against a running 3.12 install, before vs. after, over 37 behavioural probes.

Fixed — the clipper's special case becomes unnecessary. Clipper bounds,
projections and clipped-masks are identical across 3 clipper configurations × 3 input
vectors once the type switch is deleted. That equivalence is the whole point.

Fixed — prior passing no longer produces a prior that samples negative values.
mapper_from_prior_means on a LogGaussian parameter, where config supplies no
Limits entry:

before after
passed prior TruncatedGaussian(1.0, 0.5, -inf, inf) TruncatedGaussian(1.0, 0.5, 0.0, inf)
value_for(0.001) -0.545 0.0089
log_prior_from_value(-1.0) -8.0 (finite) -inf

This is downstream-visible: it changes the unit-cube mapping of a passed LogGaussian
prior. See "API Changes".

Unchanged — verified, not assumed.

  • Identifiers. Identifier(prior), its full description, and
    Identifier(LBFGS(clipper=ClipperPriorBox())) are byte-identical.
    __identifier_fields__ = ("mean", "sigma") gates it. No re-keying of output
    directories, no orphaned stored results — unlike the clipper-identifier decision
    of 2026-08-18 (complete/2026/08/clipper-in-search-identifier.md).
  • log_prior_from_value. Pointwise identical at 11 values either side of zero.
    The density was always correct; only the reported limits were wrong.
  • The nested-sampler unit-cube mapping. value_for, unit_value_for round-trip
    and Model.vector_from_unit_vector identical across a 10-point unit grid. The
    message stack is untouched, so no stored nested-sampling result shifts.
  • EP / Laplace. message.lower_limit is deliberately left at ±inf, so
    MeanField.lower_limit and LaplaceOptimiser(check_limits=True) see exactly what
    they see today. This is why the declaration goes on the prior, not the message.
  • Other prior families. GaussianPrior, UniformPrior, LogUniformPrior,
    TruncatedGaussianPrior limits all unchanged.

Net: 34 of 37 probes identical; the 3 that moved are lower_limit under copy,
pickle and project — the fix itself.

API Changes

  • New public attributes on Prior: lower_limit_strict / upper_limit_strict
    (both False), True for LogGaussianPrior.lower_limit_strict.
  • LogGaussianPrior.lower_limit now 0.0 (was -inf); limits now (0.0, inf).
  • Prior.limits now derives from lower_limit/upper_limit instead of returning
    a hardcoded (-inf, inf). GaussianPrior is unaffected.
  • Downstream: prior passing on a LogGaussian parameter without a config Limits
    entry now yields a lower-bounded truncated gaussian, changing its unit-cube mapping.
    PyAutoGalaxy / PyAutoLens inter-phase prior passing should be spot-checked.
Detailed implementation plan

Work Classification

Library — PyAutoFit only. No workspace changes.

Affected Repositories

  • PyAutoFit (primary)

Branch Survey

Repository Current Branch Dirty?
./PyAutoFit main @ 9ddf25c clean

Remote branches: main, experiment/jax-vmap-jit-ordering. No conflicting feature
branch; no active.md worktree claim on PyAutoFit.

Suggested branch: feature/loggaussian-prior-support

Worktree root: ~/Code/PyAutoLabs-wt/loggaussian-prior-support/ (created later by
/start_library; this session runs web-github against a direct clone).

Implementation Steps

  1. autofit/mapper/prior/abstract.py — add class attributes lower_limit_strict = False
    and upper_limit_strict = False to Prior, documented as "does the support
    exclude this bound". Class attributes, so they never reach __getattr__ and never
    enter __dict__ (identifier-safe).
  2. autofit/mapper/prior/abstract.py — change the Prior.limits property to return
    (float(self.lower_limit), float(self.upper_limit)).
  3. autofit/mapper/prior/log_gaussian.py — set self.lower_limit = 0.0 and
    self.upper_limit = float("inf") in __init__; add class attribute
    lower_limit_strict = True. Leave tree_flatten, _new_for_base_message and
    __identifier_fields__ alone — the limits are derived constants, not parameters,
    which is what makes them survive the JAX pytree round-trip.
  4. autofit/non_linear/clipper.py — in _limits_from_model, replace the
    isinstance(prior, LogGaussianPrior) block with
    low_strict = bool(prior.lower_limit_strict) /
    high_strict = bool(prior.upper_limit_strict); drop the now-unused import; rewrite
    the docstring paragraph that documents the workaround and the class docstring's
    "Half-open and exclusive" bullet (reframed from "declared here" to "read off the
    prior").
  5. test_autofit/non_linear/test_clipper.py — rewrite
    test__log_gaussian_prior__lower_bound_is_declared_by_the_clipper. It currently
    asserts prior.lower_limit == -np.inf, i.e. it encodes this bug as expected
    behaviour
    ; its docstring says so explicitly. Rename to reflect that the prior now
    declares its own bound, and keep the resulting bounds assertions unchanged — they
    already pass.
  6. test_autofit/mapper/prior/test_log_gaussian.py — add the reported-support
    assertions, log_prior_from_value pinned against pre-change values across
    [-3, 0, 1e-12, 0.1, 1, 10], a value_for unit-grid regression, and an identifier
    stability assertion.
  7. test_autofit/mapper/prior/test_prior_properties.py — add the general property
    (P6): for every prior family, log_prior_from_value is finite strictly inside the
    reported limits, -inf outside them, and -inf at a bound flagged strict. This
    is the general form of the bug and is what would have caught it.

Key Files

  • autofit/mapper/prior/abstract.pyPrior.__getattr__ delegation, limits,
    the new strictness contract.
  • autofit/mapper/prior/log_gaussian.py — the prior that misreports.
  • autofit/messages/composed_transform.py:109-110 — where the ±inf default
    originates (read-only; deliberately not modified, see below).
  • autofit/non_linear/clipper.py:255-285 — the workaround being retired.

Deliberately out of scope

  • Declaring the limits on TransformedMessage instead. Rejected: they would be
    dropped by with_base, copy, project and __call__, and would change what
    MeanField.lower_limit and LaplaceOptimiser(check_limits=True) feed to
    OptimisationState.valid — a live EP behaviour change well outside a bug fix.
    Filed as a follow-up.
  • Deleting the now-redundant limits overrides on UniformPrior,
    LogUniformPrior and TruncatedGaussianPrior. They are exact duplicates of the new
    base implementation, but removing them widens this PR across three more files for
    zero behavioural change. Trivial follow-up.
  • line_search.OptimisationState.valid uses if self.lower_limit and ..., which
    is falsy at 0.0. Pre-existing; untouched here. Follow-up.

Validation

  • Full suite: NUMBA_CACHE_DIR=... MPLCONFIGDIR=... python -m pytest test_autofit/
    (baseline on main: 2124 passed, 36 skipped).
  • The before/after probe harness described under "Measured impact".

Original Prompt

Click to expand starting prompt

LogGaussianPrior misreports its own support as (-inf, inf)

Type: bug
Target: autofit
Repos:

  • PyAutoFit
    Difficulty: small
    Autonomy: supervised
    Priority: normal
    Status: formalised
    Filed: 2026-08-16 (backfilled from git)

Filed 2026-08-16. Follow-up 3 owed by the prior-support Clipper
(complete/2026/08/prior-support-clipper.md, PyAutoFit#1477), which worked
around it rather than fixing it.

The defect

LogGaussianPrior's support is (0, inf)log_prior_from_value returns
-inf for value <= 0. But its TransformedMessage defaults its limits to
±inf and is never passed any, so the prior reports (-inf, inf).

Every other prior answers lower_limit / upper_limit truthfully via
Prior.__getattr__ delegating to the message, which is why the Clipper needs
no type switch anywhere else. This one prior is the exception, and it is the
kind of exception that is invisible until something trusts the answer.

Why it matters now

ClipperPriorBox declares the real support in the clipper rather than on
the prior — deliberately, to avoid touching a shared class late in that task,
and recorded as a follow-up rather than left silent. That special case is
correct but misplaced: any future consumer of lower_limit gets the wrong
answer unless it also knows to special-case this prior.

The general hazard: a bound of -inf on a strictly positive parameter means a
consumer will not guard 0, and log(0) / a division by it is the failure that
follows.

The fix

Declare the support on LogGaussianPrior itself — pass the limits into the
TransformedMessage, or override lower_limit — then retire the clipper's
special case and its accompanying comment.

The care needed — why this is supervised and not safe

Changing what a prior reports as its support is not local:

  • The nested samplers work in unit-cube coordinates and map through the
    prior. Confirm a limits change does not alter that mapping, or every stored
    nested-sampling result shifts.
  • log_prior_from_value must not change behaviour. It is already correct;
    only the reported limits are wrong. If the fix changes the density anywhere,
    it has gone too far.
  • Check the identifier. If lower_limit feeds the search identifier, a
    change re-keys existing output directories and orphans stored results — the
    same class of concern as the clipper identifier decision, which chose to
    re-key and orphan rather than special-case (2026-08-18; record
    complete/2026/08/clipper-in-search-identifier.md).

Verify

  • LogGaussianPrior(...).lower_limit == 0.0 (or whatever exclusive convention
    is chosen — state it).
  • log_prior_from_value is unchanged across a range of values either side of
    zero, asserted against the pre-change values.
  • ClipperPriorBox.bounds_from_model returns the same bounds for a model
    containing a LogGaussianPrior after the clipper's special case is
    removed as it did before — that equivalence is the whole point of the change.
  • A nested-sampler unit-cube round-trip through the prior is unchanged.

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