fix: correct erfc_inv's sign error and keep evidence in log space
A systematic scan for precision defects, following the tail-precision work in7341669. Three findings; the first is a correctness bug in a released version. 1. `erfc_inv`'s initial guess had the wrong sign. Numerical Recipes' `inverfc` uses -0.70711 as the leading coefficient; this used +FRAC_1_SQRT_2. Since `rational - t` is negative, that put Newton on the mirror image of the root, and three fixed iterations could not cross back. Measured against exact standard-normal quantiles: p_draw old rel err new rel err 0.50 1.46e-7 8.40e-8 0.90 1.02e-1 8.63e-9 0.95 3.06e-1 1.91e-8 0.99 8.05e-1 5.89e-9 `compute_margin` inherited it, so the draw margin was wrong for any `p_draw` above about 0.6 and *non-monotone* above 0.9 — it ran 0.674, 1.476, 0.503, 0.982 as p_draw went 0.5, 0.9, 0.99, 0.999. A history configured for a 0.99 draw rate was being fitted at 0.385. Note it was slightly wrong everywhere, not only in the tail. 2. `MarginFactor` computed a density and clamped it. `pdf` underflows past ~38 sigma, so `ln` of the clamped zero reported -708 nats however far out the score actually was: 4292 nats adrift at 100 sigma, and unbounded beyond. This is the same defect as the one fixed in `TruncFactor`, one file over, on the scored-outcome path. 3. `TruncFactor` still bottomed out past ~38 sigma even after7341669removed the cancellation, because the linear probability itself underflows there. 2 and 3 are fixed the same way: factors cache a *log* evidence, built from new `ln_pdf`, `ln_sf` and `ln_interval` helpers that factor the shared exponential out analytically via the `erfcx` added earlier. Nothing underflows, at any separation. One golden moved. `test_1vs1vs1` runs at `p_draw = 0.5`, so it goes through `compute_margin`; its 1e-6-place values shifted. Verified as movement *toward* analytic truth by comparing both the old and new inverse against exact quantiles, per the goldens policy in CLAUDE.md — not re-baselined on faith. Two test tolerances are asserted at 1e-6 rather than tighter because above x = 2 `erfcx` uses a continued fraction accurate to ~1e-15 while `erfc` carries ~1e-7, so the log path is the more accurate of the two and they part company at `erfc`'s error. That floor is tracked in #41. Refs #41 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
+28
-16
@@ -2,7 +2,7 @@ use crate::{
|
||||
N_INF,
|
||||
factor::{Factor, VarId, VarStore},
|
||||
gaussian::Gaussian,
|
||||
pdf,
|
||||
ln_pdf,
|
||||
};
|
||||
|
||||
/// Gaussian observation factor on a diff variable.
|
||||
@@ -16,7 +16,7 @@ pub struct MarginFactor {
|
||||
pub m_obs: f64,
|
||||
pub sigma: f64,
|
||||
pub(crate) msg: Gaussian,
|
||||
pub(crate) evidence_cached: Option<f64>,
|
||||
pub(crate) log_evidence_cached: Option<f64>,
|
||||
}
|
||||
|
||||
impl MarginFactor {
|
||||
@@ -28,7 +28,7 @@ impl MarginFactor {
|
||||
m_obs,
|
||||
sigma,
|
||||
msg: N_INF,
|
||||
evidence_cached: None,
|
||||
log_evidence_cached: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -41,8 +41,8 @@ impl MarginFactor {
|
||||
let marginal = vars.get(self.diff);
|
||||
let cavity = marginal / self.msg;
|
||||
|
||||
if self.evidence_cached.is_none() {
|
||||
self.evidence_cached = Some(cavity_evidence(cavity, self.m_obs, self.sigma));
|
||||
if self.log_evidence_cached.is_none() {
|
||||
self.log_evidence_cached = Some(cavity_log_evidence(cavity, self.m_obs, self.sigma));
|
||||
}
|
||||
|
||||
let new_msg = Gaussian::from_ms(self.m_obs, self.sigma);
|
||||
@@ -61,17 +61,29 @@ impl Factor for MarginFactor {
|
||||
}
|
||||
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
self.evidence_cached.unwrap_or(1.0).ln()
|
||||
self.log_evidence_cached.unwrap_or(0.0)
|
||||
}
|
||||
}
|
||||
|
||||
/// Density of the observed margin under the cavity, clamped to a positive
|
||||
/// floor so a far-out observation cannot underflow to `0.0` and make
|
||||
/// `log_evidence` `-inf`.
|
||||
fn cavity_evidence(cavity: Gaussian, m_obs: f64, sigma: f64) -> f64 {
|
||||
/// `ln` of the observed margin's density under the cavity.
|
||||
///
|
||||
/// Computed in log space rather than as `pdf(..).ln()`. The density underflows
|
||||
/// to zero past about 38 sigma of separation, and clamping that to
|
||||
/// `f64::MIN_POSITIVE` reported -708 nats however far out the observation
|
||||
/// actually was — 4292 nats adrift at 100 sigma, and unbounded beyond. A score
|
||||
/// far from what the model expected is exactly the observation a log-evidence
|
||||
/// figure exists to notice.
|
||||
fn cavity_log_evidence(cavity: Gaussian, m_obs: f64, sigma: f64) -> f64 {
|
||||
let combined_sigma = (cavity.sigma().powi(2) + sigma.powi(2)).sqrt();
|
||||
let value = ln_pdf(m_obs, cavity.mu(), combined_sigma);
|
||||
|
||||
pdf(m_obs, cavity.mu(), combined_sigma).max(f64::MIN_POSITIVE)
|
||||
// A degenerate cavity (infinite sigma) is the only way to reach a
|
||||
// non-finite result; fall back to the old floor rather than emit -inf.
|
||||
if value.is_finite() {
|
||||
value
|
||||
} else {
|
||||
f64::MIN_POSITIVE.ln()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -113,16 +125,16 @@ mod tests {
|
||||
let mut vars = VarStore::new();
|
||||
let diff = vars.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f = MarginFactor::new(diff, 5.0, 1.0);
|
||||
assert!(f.evidence_cached.is_none());
|
||||
assert!(f.log_evidence_cached.is_none());
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let z = f.evidence_cached.unwrap();
|
||||
// pdf(5, 0, sqrt(37)) ≈ 0.046783
|
||||
assert!((z - 0.04678300292616668).abs() < 1e-10);
|
||||
let z = f.log_evidence_cached.unwrap();
|
||||
// ln pdf(5, 0, sqrt(37)) = ln(0.046783...)
|
||||
assert!((z.exp() - 0.04678300292616668).abs() < 1e-10);
|
||||
|
||||
// Subsequent propagations don't change it.
|
||||
f.propagate(&mut vars);
|
||||
assert_eq!(f.evidence_cached.unwrap(), z);
|
||||
assert_eq!(f.log_evidence_cached.unwrap(), z);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user