Repository navigation
Use exact intercept term A^-1 (exp(A t) - I) b - #126
Merged
Merged
Conversation
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>
…ed parameters Stan block
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 byb:That matrix is only symmetric when
Ais, 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
05-transformed-parameters.stan(Stan) and_compute_caches()(JAX). TheVCV_cachesymmetrisation stays, since that matrix is a covariance.tests/testthat/test-ou_mean.R. It sets drift to zero, then checks that the tip means equal the exact OU solutionmu + exp(A)(eta_anc - mu)on an ultrametric tree with asymmetricAand non-zerob. 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:b = 0;Impact
bis weakly identified on an ultrametric tree, and it absorbs most of the error.AandQbb = 0Checks
NOT_CRAN=true): 0 failures.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