Skip to content

fix(graphical): EP statistics fix batch — F1/F2/F4/F8 from #1332#1351

Merged
Jammy2211 merged 1 commit into
mainfrom
feature/ep-statistics-fix-batch
Jul 10, 2026
Merged

fix(graphical): EP statistics fix batch — F1/F2/F4/F8 from #1332#1351
Jammy2211 merged 1 commit into
mainfrom
feature/ep-statistics-fix-batch

Conversation

@Jammy2211

Copy link
Copy Markdown
Collaborator

Summary

Executes the remainder of the #1332 EP statistics audit's recommended fix batch (tracker #1350), now that the #1331 decision hub is resolved (#1345, #1348) and F3/F10 (#1349), F5 (#1334), F7(c) (#1345) have landed. Two real statistical repairs — MeanField evidence bookkeeping through __truediv__/__pow__ (F1/F7a: log_norm was silently dropped into the plates ctor slot) and the Gamma/Beta KL direction (F2: both computed KL(other‖self) while Normal/TruncatedNormal compute KL(self‖other), so EPHistory.kl_divergence summed opposite-direction KLs on mixed graphs) — plus one exoneration (F4: the self-flagged update_invalid scalar branch is empirically correct; stale TODO replaced and both branches pinned by tests) and one deletion (F8: three dead/suspect quasi-Newton variants).

API Changes

Behaviour fixes on EP internals; three never-exported module functions removed.
GammaMessage.kl / BetaMessage.kl now return KL(self‖other) (family-wide contract) — EP convergence metrics on graphs with Gamma/Beta variables change value (correctly). MeanField.__truediv__/__pow__ now propagate log_norm, so EPMeanField.log_evidence through these operators is no longer silently zeroed (one leg of F7's "evidence bookkeeping broken"; F7(b) — where sampler per-factor evidence is recorded — remains a parked design decision).
See full details below.

Test Plan

  • Full test_autofit/ suite: 1465 passed / 14 skipped / 1 pre-existing flaketest_full_hierachical fails on clean main with this batch fully stashed (bisect-proven ×3: clean, F1-reverted, F2-reverted); filed as PyAutoMind/bug/autofit/hierarchical_ep_test_flaky.md
  • New test_ep_statistics_fixes.py — 9 tests: log_norm/plates round-trip through __truediv__/__pow__ (scalar + MeanField exponent), Monte-Carlo KL direction property test per family (Normal/Gamma/Beta, asymmetric cases, reverse direction asserted different), update_invalid scalar + array branches, dead-variant removal
  • EP integration smoke: ep_parity.py / ep_deterministic.py / ep_exact.py all PASS against this branch

Validation checklist (--auto run — plan on the issue)

Full API Changes (for automation & release notes)

Removed

  • autofit.graphical.laplace.newton.diag_sr1_update_ — never referenced
  • autofit.graphical.laplace.newton.diag_sr1_bfgs_update — computed locals then implicitly returned None (would have nuked optimiser state if wired in)
  • autofit.graphical.laplace.newton.bfgs1_update — disagreed with the exported bfgs_update on the y_k sign and carried a stray minus on d_k^T B d_k
    (None were exported via autofit.graphical; the verified bfgs_update/sr1_update/full_* variants are unchanged.)

Changed Behaviour

  • MeanField.__truediv__log_norm now correctly self.log_norm - other.log_norm (previously silently 0.0 with the value stored as a float in _plates)
  • MeanField.__pow__ (scalar) — log_norm = self.log_norm * exponent in the right slot; (MeanField exponent) — log_norm = 0.0 with rationale comment (the previous self.log_norm * other.log_norm was not a meaningful quantity)
  • GammaMessage.kl(dist) / BetaMessage.kl(dist) — now KL(self‖dist); previously KL(dist‖self). Convergence metric values on mixed-family EP graphs change; the EP fixed point itself is unaffected (KL is only used for history/convergence checks)

Migration

  • None. No public exports removed; code that (incorrectly) relied on the reversed Gamma/Beta KL direction should swap caller/argument.

🤖 Generated with Claude Code

- F1/F7(a): MeanField.__truediv__/__pow__ passed log_norm positionally
  into the plates ctor slot — evidence silently dropped and a float
  stored in _plates. log_norm now passed by keyword; the per-variable-
  exponent __pow__ branch sets log_norm=0.0 (no meaningful scalar
  aggregate exists; the old self.log_norm * other.log_norm was
  meaningless even in the right slot).
- F2 (+extended): GammaMessage.kl and BetaMessage.kl computed the
  reverse direction KL(dist||self); both now follow the family-wide
  contract self.kl(other) = KL(self||other) (matching Normal /
  TruncatedNormal), so EPHistory.kl_divergence no longer sums KLs
  measured in opposite directions on mixed graphs. Certified per family
  by a Monte-Carlo direction property test.
- F4: AbstractMessage.update_invalid scalar branch empirically verified
  correct — the "fairly certain this would not work" TODO was a stale
  false alarm; replaced with an explanatory comment and pinned by unit
  tests on both branches.
- F8: dead/suspect quasi-Newton variants deleted from laplace/newton.py
  (diag_sr1_update_ unused; diag_sr1_bfgs_update returned None;
  bfgs1_update sign-disputed vs the exported bfgs_update).

Adds test_ep_statistics_fixes.py (9 tests). Full suite: 1465 passed /
14 skipped / 1 pre-existing flake (test_full_hierachical — fails on
clean main with this batch fully stashed; filed as
PyAutoMind/bug/autofit/hierarchical_ep_test_flaky.md).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Jammy2211 Jammy2211 added the pending-release PR queued for the next release build label Jul 10, 2026
@Jammy2211 Jammy2211 merged commit c5c50cc into main Jul 10, 2026
2 of 6 checks passed
@Jammy2211 Jammy2211 deleted the feature/ep-statistics-fix-batch branch July 10, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending-release PR queued for the next release build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant