You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Design discussion only — no code until this is approved. This is the bundled design issue for the last two structural items of the priors/messages census (hub #1331 "parked" list): the single-source-of-truth density refactor (census C1, prompt bug/priors/12) and the Prior/Message hierarchy collapse (census C4, prompt bug/priors/13). Both were deliberately sequenced behind Phases 1–2 of the EP framework review; that gate opened in July — the EP statistics audit (#1332, findings F1–F10) and the formal autofit/graphical/README.md (#1334, 16 code-anchored equations) are the design inputs this issue builds on. Maintainer + contributor input wanted, especially anyone with exponential-family / EP background (@rhayes777).
Why now — the evidence base
Four confirmed bug families in 13 months trace to the same structural cause: each density is encoded in several places with independent conventions, split across two class hierarchies joined by __getattr__.
GaussianPrior's class hierarchy defines 28 public methods; the NormalMessage it wraps defines 53; only 5 overlap (factor, log_normalisation, ndim, project, value_for).
48 message methods are reachable on the prior only via Prior.__getattr__ — invisible on the prior class, in stack traces, and to static analysis. That set includes log_prior_from_value, the function every MCMC/MLE search calls in its hot loop.
The consumer split, measured
Grepping call sites by package (files containing .method():
The split the prompts hypothesised is real and clean: EP-specific operations are a coherent, separable set, and the user-facing set is small. Notably, almost nothing outside the messages package calls logpdf directly (1 file each in graphical and mapper) — relevant to the #1498 contract decision.
Proposed direction (sketch to react to, not a fait accompli)
One hierarchy, two functions per family, EP as a mixin:
Distribution (replaces both Prior and Message)
├── log_density(x, xp=np) — THE density. The one place the math lives.
├── value_for(unit, xp=np) — inverse CDF (no closed-form derivation from
│ log_density; each family implements it)
├── logpdf / log_prior_from_value — one-line aliases of log_density
│ (log_prior_from_value keeps the #1331 Option A
│ contract: constants dropped + log_normalisation())
├── Univariate families: Uniform, LogUniform, Gaussian, LogGaussian,
│ TruncatedGaussian, Beta, Gamma
└── EPMixin (natural_parameters, log_partition, project, kl,
merge, from_mode, sufficient stats, …)
— applied to families that participate in factor graphs; the exact member
list is the measured EP-only set above, cross-checked against
autofit/graphical/README.md §§2–3 (projection, moment matching, cavity).
width_modifier, id_, model-mapper integration move onto the family classes; __getattr__ delegation is deleted.
Transforms: TransformedMessage becomes an internal composition detail of the transformed families, with one physical-density path (resolving TransformedMessage.logpdf/pdf omit the transform Jacobian (base-space density at physical coordinates) #1498 by construction). Whether its innards then migrate to a bijector library is prompt bug/priors/14 — that go/no-go deliberately hangs off this issue's outcome and is out of scope here.
User-visible API (af.GaussianPrior(...), value_for, prior_passing, with_limits) is preserved; old names become deprecated aliases for one release cycle.
Migrate one family per PR (Gaussian → Uniform/LogUniform → LogGaussian → TruncatedGaussian → Beta/Gamma), moving each family's EP operations onto the mixin as it migrates.
DeprecatePrior.__getattr__, the old Message entry points, and the duplicate density encodings; remove after one release cycle.
The #1497 property sweep is the safety net that makes this refactor tractable — every stage must keep all 134 property tests green, so the corrected definitions from #1345/#1348 cannot silently regress.
Decisions wanted
One hierarchy or two? Merge Prior+Message into Distribution + EP mixin (prompt 13's position), or keep two layers but with a single shared log_density (prompt 12 alone)? The measured consult split above supports the merge; the counterargument is churn in autofit/graphical internals that research: EP review phase 1 — statistics audit of autofit/graphical #1332 just verified correct.
Design discussion only — no code until this is approved. This is the bundled design issue for the last two structural items of the priors/messages census (hub #1331 "parked" list): the single-source-of-truth density refactor (census C1, prompt
bug/priors/12) and the Prior/Message hierarchy collapse (census C4, promptbug/priors/13). Both were deliberately sequenced behind Phases 1–2 of the EP framework review; that gate opened in July — the EP statistics audit (#1332, findings F1–F10) and the formalautofit/graphical/README.md(#1334, 16 code-anchored equations) are the design inputs this issue builds on. Maintainer + contributor input wanted, especially anyone with exponential-family / EP background (@rhayes777).Why now — the evidence base
Four confirmed bug families in 13 months trace to the same structural cause: each density is encoded in several places with independent conventions, split across two class hierarchies joined by
__getattr__.log_prior_from_valuesign convention drifted from the other density paths (4-year-old bug).TruncatedNormalMessagegeneric-path pdf integrated to 2.27 while its direct paths were correct.TransformedMessage.logpdf/pdfreturn the base-space density at physical coordinates (UniformPrior(0,1).message.pdfintegrates to 1/(2√π)) whilefactor()is correct — a fourth divergent encoding of "the density of a transformed distribution".The delegation surface, measured (main @
7d4d931)GaussianPrior's class hierarchy defines 28 public methods; theNormalMessageit wraps defines 53; only 5 overlap (factor,log_normalisation,ndim,project,value_for).Prior.__getattr__— invisible on the prior class, in stack traces, and to static analysis. That set includeslog_prior_from_value, the function every MCMC/MLE search calls in its hot loop.The consumer split, measured
Grepping call sites by package (files containing
.method():autofit/graphical(EP) onlymapper/non_linear(user-facing)kl(5 files),merge(6),logpdf_gradient,calc_exact_update,has_exact_projection,from_mode,update_invalid, natural-parameter arithmetic (sum_/sub_natural_parameters, via operators)value_for(16 files),cdf(5),project(3),log_prior_from_value(2),sample,zeros_likeThe split the prompts hypothesised is real and clean: EP-specific operations are a coherent, separable set, and the user-facing set is small. Notably, almost nothing outside the messages package calls
logpdfdirectly (1 file each ingraphicalandmapper) — relevant to the #1498 contract decision.Proposed direction (sketch to react to, not a fait accompli)
One hierarchy, two functions per family, EP as a mixin:
width_modifier,id_, model-mapper integration move onto the family classes;__getattr__delegation is deleted.TransformedMessagebecomes an internal composition detail of the transformed families, with one physical-density path (resolving TransformedMessage.logpdf/pdf omit the transform Jacobian (base-space density at physical coordinates) #1498 by construction). Whether its innards then migrate to a bijector library is promptbug/priors/14— that go/no-go deliberately hangs off this issue's outcome and is out of scope here.af.GaussianPrior(...),value_for,prior_passing,with_limits) is preserved; old names become deprecated aliases for one release cycle.Staged migration (each stage independently shippable)
Distributionas a sibling layer implementinglog_density/value_forfor one family (Gaussian), with the existing classes delegating into it — no behaviour change, property tests (test: property-based correctness sweep over every Prior subclass #1497/PR test: property-based correctness sweep over every Prior subclass #1499) green throughout.TransformedMessageinto the transformed families with a single physical-density path (TransformedMessage.logpdf/pdf omit the transform Jacobian (base-space density at physical coordinates) #1498 resolved by design here at the latest).Prior.__getattr__, the oldMessageentry points, and the duplicate density encodings; remove after one release cycle.The #1497 property sweep is the safety net that makes this refactor tractable — every stage must keep all 134 property tests green, so the corrected definitions from #1345/#1348 cannot silently regress.
Decisions wanted
Distribution+ EP mixin (prompt 13's position), or keep two layers but with a single sharedlog_density(prompt 12 alone)? The measured consult split above supports the merge; the counterargument is churn inautofit/graphicalinternals that research: EP review phase 1 — statistics audit of autofit/graphical #1332 just verified correct.TransformedMessage.logpdfbe the physical density (addlog_det, matching the docs: formal Bayesian specification of autofit.graphical (EP review phase 2) #1334 docstring) or documented as base-space for EP arithmetic? The unified design makes this moot eventually, but the interim fix needs the call.graphical/README.md§3.3?This is a months-long staged migration, not a single PR. Nothing starts until the direction is agreed here.