Skip to content

Fix -wslr/-wslm .sitelh invariant under per-category (safe_numeric) scaling#534

Open
famulare wants to merge 1 commit into
iqtree:masterfrom
famulare:fix-wslr-sitelh-invariant
Open

Fix -wslr/-wslm .sitelh invariant under per-category (safe_numeric) scaling#534
famulare wants to merge 1 commit into
iqtree:masterfrom
famulare:fix-wslr-sitelh-invariant

Conversation

@famulare

@famulare famulare commented Jul 7, 2026

Copy link
Copy Markdown

Fixes the .sitelh invariant documented in the file's own header:

#   LnLW_k: Logarithm of (category-k site likelihood times category-k weight)
#           Thus, sum of exp(LnLW_k) is equal to exp(LnL)

Fixes #533. See that issue for the full root-cause write-up and reproduction.

The bug

PhyloTree::computePatternLikelihood() writes the per-rate/mixture-category
site log-likelihoods (-wslr/-wslm/-wslmr). It chose between per-category
and per-pattern scaling with:

if (params->lk_safe_scaling || leafNum >= params->numseq_safe_scaling)

But the likelihood kernel decides its scaling mode via PhyloTree::safe_numeric
(set in setLikelihoodKernel()), which is broader:

safe_numeric = (params && (params->lk_safe_scaling || leafNum >= params->numseq_safe_scaling)) ||
    (aln && aln->num_states != 4 && aln->num_states != 20);

The two conditions differ only on the num_states clause, so they disagree
exactly when the writer condition is false but safe_numeric is true: a
non-4-state-DNA / non-20-state-protein alphabet (codon, morphology, custom),
run without -safe, on a tree below numseq_safe_scaling (default
2000) tips. In that case the kernel scales per category and stores
scale_num per category (scale_num[ptn*ncat_mix + c]), while the writer took
its per-pattern branch and read scale_num as a flat per-pattern array
(scale_num[ptn]). This mis-indexes the scale counts and applies a single,
wrong scale step uniformly to all categories. On numerically-extreme sites
(categories underflowing at different scale counts) every LnLW_k comes out
exactly LOG_SCALING_THRESHOLD (≈ 177.4457) too low, breaking the invariant.
(-safe or a ≥2000-tip tree make the writer condition true as well, so they
mask the bug rather than cause it.) .siteprob is unaffected because its
posteriors are normalized.

The fix

Use the tree's own safe_numeric flag to select the writer branch, so the
writer always matches the kernel's actual scaling layout.

         double *lh_cat = _pattern_lh_cat;
         double *out_lh_cat = ptn_lh_cat;
         UBYTE *nei2_scale = nei2->scale_num;
-        if (params->lk_safe_scaling || leafNum >= params->numseq_safe_scaling) {
+        // NOTE: must match the kernel's per-category vs per-pattern scaling choice
+        // (PhyloTree::safe_numeric, set in setLikelihoodKernel), otherwise scale_num
+        // is mis-indexed and the per-category log-likelihoods are off by a scale step.
+        if (safe_numeric) {
             // per-category scaling

(same change in the external-branch and internal-branch blocks; two if
conditions).

Verification

Reproduction (codon, deep branches, 5 well-separated FreeRate categories,
seed 7 — fully self-contained via --alisim, see the companion issue for the
commands and the verification script):

build rows violations (|dev|>1e-3) max |dev|
2.4.0 (before) 667 18 177.445710
this PR (after) 667 0 0.000096

Example site 85, before → after:

before:  LnL=-3.8118   LnLW = -181.9381 -182.2600 -183.3261 -188.9624 -215.2784
after:   LnL=-3.8118   LnLW =   -4.4925   -4.8143   -5.8805  -11.5167  -37.8327

Regression / no-change checks:

  • DNA (GTR+F+R4, 4-state, per-pattern path): .sitelh is
    byte-identical before and after the patch.
  • Protein path is likewise on the unchanged (per-pattern) branch.
  • Standard end-to-end run on the bundled example/example.phy
    (GTR+R4 -wslr): invariant holds (max|dev| ≈ 9e-5).
  • Morphology (MK+R) and profile-mixture (-wslmr) codon runs: invariant
    holds (0 violations).
  • Total tree log-likelihood is unchanged (-10226.6026 before and after),
    and sum_i LnL_i still equals the tree log-likelihood — confirming the
    change touches only the per-category reporting, not the likelihood engine.

Scope

Two-line logic change in tree/phylotree.cpp (plus comments). No behavior
change for DNA/protein; no change to the likelihood engine, tree search, or
any other output.


AI-assistance disclosure: this fix was diagnosed and drafted with the help
of Claude Opus 4.8. All changes were reviewed and verified by Mike Famulare
before submission.

writeSiteLh (via computePatternLikelihood) documents the invariant
"sum of exp(LnLW_k) is equal to exp(LnL)". It is violated on
numerically-extreme sites whenever the likelihood kernel runs with
per-category scaling (safe_numeric == true) but the per-category
writer takes its per-pattern scaling branch.

The kernel sets safe_numeric in setLikelihoodKernel() as:
    (lk_safe_scaling || leafNum >= numseq_safe_scaling)
        || (num_states != 4 && num_states != 20)
i.e. codon / morphology / other non-DNA-non-protein alphabets always
use per-category scaling and lay scale_num out per category
(scale_num[ptn*ncat_mix + c]).

computePatternLikelihood() chose its per-category-vs-per-pattern branch
using only (lk_safe_scaling || leafNum >= numseq_safe_scaling), omitting
the num_states clause. So for e.g. a 61-state codon model the writer
read scale_num as a per-pattern array (scale_num[ptn]), mis-indexing it
and applying a single wrong scale step uniformly to all categories. On
sites deep enough for the categories to underflow at different scale
counts, every LnLW_k came out exactly LOG_SCALING_THRESHOLD
(-177.4457) too low, so sum exp(LnLW_k) < exp(LnL).

Fix: select the writer branch with the tree's own safe_numeric flag,
which is the authoritative record of the kernel scaling path used.
DNA/protein (4/20-state) output is byte-identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

-wslr/-wslm .sitelh violates its own sum(exp(LnLW_k)) == exp(LnL) invariant for codon/morphology models

1 participant