Skip to content

Use exact intercept term A^-1 (exp(A t) - I) b - #126

Merged
ScottClaessens merged 5 commits into
mainfrom
fix-intercept-symmetrisation
Oct 6, 2026
Merged

ScottClaessens merged 5 commits into
mainfrom
fix-intercept-symmetrisation

Conversation

@ErikRingen

@ErikRingen ErikRingen commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

When branch-length caching was added (efcb253, v1.0.0), A_solve_cache = A^-1 (exp(A t) - I) was symmetrised before being multiplied by b:

for (i in 1:J) for (j in 1:i) {
  real val = 0.5 * (A_solve_cache[u][i, j] + A_solve_cache[u][j, i]);
  ...
}

That matrix is only symmetric when A is, so models with asymmetric cross-selection effects used an incorrect expected change along each branch. The JAX backend copied the same step. Every branch uses the cached path, because all branch lengths are cached. The uncached fallback was always exact.

Change

  • Remove the symmetrisation in 05-transformed-parameters.stan (Stan) and _compute_caches() (JAX). The VCV_cache symmetrisation stays, since that matrix is a covariance.
  • Add tests/testthat/test-ou_mean.R. It sets drift to zero, then checks that the tip means equal the exact OU solution mu + exp(A)(eta_anc - mu) on an ultrametric tree with asymmetric A and non-zero b. The test fails on main (e.g. 0.52 vs 0.86) and passes with this change. None of the existing tests could catch this:
    • the recovery test fixes b = 0;
    • the fixture tests only check that fits run;
    • the Stan/JAX comparisons check two implementations that shared the step.

Impact

b is weakly identified on an ultrametric tree, and it absorbs most of the error.

Comparison (current vs fixed code) A and Q b
Authority example ≤ 0.1 posterior SD ≤ 0.1 posterior SD
Simulated normal + Bernoulli, 2 scenarios indistinguishable indistinguishable
Synthetic cichlid example from Ringen et al. (2026), data simulated with b = 0 ≤ 0.08 posterior SD 0.39–0.63 posterior SD; all three means move closer to the true value of 0, intervals slightly wider (SD 0.50–0.72 → 0.60–0.83). One simulated dataset.

Checks

  • Default test suite (NOT_CRAN=true): 0 failures.
  • Extended suite (COEVOLVE_EXTENDED_TESTS=true): direction, recovery, scaling, prior and the full Stan/JAX log-density suite (21 configurations), all passing.

This is independent of #125 and applies cleanly on top of it.

🤖 Generated with Claude Code

ErikRingen and others added 3 commits October 5, 2026 16:06
The branch-length caching added in efcb253 symmetrised A_solve_cache
before multiplying by b. A^-1 (exp(A t) - I) is only symmetric when A
is, so models with asymmetric cross-selection effects used an incorrect
expected change along each branch. The JAX backend mirrored the same
step. Both backends now use the exact expression.

Adds a test that the tip means of a zero-drift model follow the exact
OU solution for asymmetric A and non-zero b. The test fails on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ErikRingen ErikRingen added the run-extended Run the extended (logp Stan/JAX) test suite in CI label Oct 5, 2026
@ScottClaessens
ScottClaessens merged commit a4289a9 into main Oct 6, 2026
13 checks passed
@ScottClaessens
ScottClaessens deleted the fix-intercept-symmetrisation branch October 6, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-extended Run the extended (logp Stan/JAX) test suite in CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants