An AI-assisted audit of autofit/mapper/prior/ and autofit/messages/ (follow-up to the LogUniform fix, #1329) found 9 real bugs plus refactor ideas. Every finding has now been independently re-verified on main @ 0f26ff2d8 — reproductions and full detail in #1330. This issue is the decision hub: maintainer guidance + contributor input wanted, especially from anyone with exponential-family / EP background (@rhayes777).
Will fix as one batch — no decision needed
LogGaussianPrior.with_limits crashes on first call (TypeError: passes lower_limit/upper_limit kwargs the ctor doesn't accept); _new_for_base_message also broken. Fix: drop the kwargs — a log-Gaussian has support (0, ∞).UniformPrior.logpdf fails on array input (scalar-only boundary snap, acknowledged # TODO). Fix: vectorise with np.where.TruncatedNormalMessage.log_partition is missing the Gaussian term — via the generic exponential-family interface the pdf integrates to 2.27, not 1.0 (error factor exactly σ·exp(μ²/2σ²)). Sampling and log_prior_from_value are unaffected; the wrong path is the one the EP machinery consumes. Fix: add the Gaussian log-partition (as NormalMessage already does).FixedMessage.logpdf_cache is an unbounded class-level dict that also returns an aliased mutable array (mutating a result corrupts later calls). Fix: np.zeros_like(x), no cache.
Decisions wanted (my recommendation in bold)
- Beta projection clamp is a no-op (
inv_beta_suffstats clamps into a local that's immediately overwritten, so negative α, β escape with only a warning). When Newton–Raphson goes negative: raise (a failed projection shouldn't be silently patched to 0.5), or make the clamp work as originally intended?
NormalMessage accepts negative sigma (the assert is commented out; TruncatedNormalMessage rejects it, so the two disagree). Enforce reject σ ≤ 0 strictly, or reject only σ < 0 and permit a σ = 0 point-mass?GammaMessage.from_mode(mode, V) builds the wrong variance — inversely wrong: request var 0.25, get 2.0; request 4.0, get 0.235 (α = 1 + m²·V has V upside-down). Which invariant should from_mode preserve: match mean + variance (α = m²/V, β = m/V — consistent with the Normal family), the α ≥ 1-guaranteed variant (α = 1 + m²/V), or solve the quadratic to match mode + variance exactly?log_prior_from_value normalisation is inconsistent: Uniform/LogUniform/Gaussian/LogGaussian drop their additive constants; TruncatedNormal returns the fully normalised density. Posterior shape is unaffected either way, but evidence/Bayes-factor arithmetic and logpdf users see silently different conventions. Option A: drop constants everywhere, document it as the contract, add an optional per-prior log-normaliser hook — or Option B: fully normalised density everywhere (scipy convention, touches every prior + JAX paths)?RelativeWidthModifier (the default prior-passing width, 0.5·mean) yields σ = 0 for means at zero and σ < 0 for negative means — silently, until (2) lands. Scope: minimal — use abs(mean) with an optional configurable absolute floor, mandatory floor with config migration, or a full width-modifier redesign?
Parked (design work, sequenced behind the EP framework review)
Property-based prior tests (after the fixes land), TransformedMessage semantics documentation, the single-source-of-truth density refactor, the Prior/Message hierarchy collapse, and bijector-library migration. These will be shaped by the EP statistics review now underway and get their own design issues later.
🤖 Generated with Claude Code
An AI-assisted audit of
autofit/mapper/prior/andautofit/messages/(follow-up to the LogUniform fix, #1329) found 9 real bugs plus refactor ideas. Every finding has now been independently re-verified onmain@0f26ff2d8— reproductions and full detail in #1330. This issue is the decision hub: maintainer guidance + contributor input wanted, especially from anyone with exponential-family / EP background (@rhayes777).Will fix as one batch — no decision needed
LogGaussianPrior.with_limitscrashes on first call (TypeError: passeslower_limit/upper_limitkwargs the ctor doesn't accept);_new_for_base_messagealso broken. Fix: drop the kwargs — a log-Gaussian has support (0, ∞).UniformPrior.logpdffails on array input (scalar-only boundary snap, acknowledged# TODO). Fix: vectorise withnp.where.TruncatedNormalMessage.log_partitionis missing the Gaussian term — via the generic exponential-family interface the pdf integrates to 2.27, not 1.0 (error factor exactly σ·exp(μ²/2σ²)). Sampling andlog_prior_from_valueare unaffected; the wrong path is the one the EP machinery consumes. Fix: add the Gaussian log-partition (asNormalMessagealready does).FixedMessage.logpdf_cacheis an unbounded class-level dict that also returns an aliased mutable array (mutating a result corrupts later calls). Fix:np.zeros_like(x), no cache.Decisions wanted (my recommendation in bold)
inv_beta_suffstatsclamps into a local that's immediately overwritten, so negative α, β escape with only a warning). When Newton–Raphson goes negative: raise (a failed projection shouldn't be silently patched to 0.5), or make the clamp work as originally intended?NormalMessageaccepts negative sigma (the assert is commented out;TruncatedNormalMessagerejects it, so the two disagree). Enforce reject σ ≤ 0 strictly, or reject only σ < 0 and permit a σ = 0 point-mass?GammaMessage.from_mode(mode, V)builds the wrong variance — inversely wrong: request var 0.25, get 2.0; request 4.0, get 0.235 (α = 1 + m²·Vhas V upside-down). Which invariant shouldfrom_modepreserve: match mean + variance (α = m²/V, β = m/V — consistent with the Normal family), the α ≥ 1-guaranteed variant (α = 1 + m²/V), or solve the quadratic to match mode + variance exactly?log_prior_from_valuenormalisation is inconsistent: Uniform/LogUniform/Gaussian/LogGaussian drop their additive constants; TruncatedNormal returns the fully normalised density. Posterior shape is unaffected either way, but evidence/Bayes-factor arithmetic andlogpdfusers see silently different conventions. Option A: drop constants everywhere, document it as the contract, add an optional per-prior log-normaliser hook — or Option B: fully normalised density everywhere (scipy convention, touches every prior + JAX paths)?RelativeWidthModifier(the default prior-passing width, 0.5·mean) yields σ = 0 for means at zero and σ < 0 for negative means — silently, until (2) lands. Scope: minimal — use abs(mean) with an optional configurable absolute floor, mandatory floor with config migration, or a full width-modifier redesign?Parked (design work, sequenced behind the EP framework review)
Property-based prior tests (after the fixes land),
TransformedMessagesemantics documentation, the single-source-of-truth density refactor, the Prior/Message hierarchy collapse, and bijector-library migration. These will be shaped by the EP statistics review now underway and get their own design issues later.🤖 Generated with Claude Code