Replace Sankoff cost matrix with cardinality-composition decomposition
Replaced the legacy Sankoff parsimony pipeline with a new cardinality-composition decomposition that constructs row-normalized transition probability matrices symmetrized via geometric mean. This ensures reversibility, reduces free parameters from 240 to 120, and guarantees a zero diagonal. Tallies are now explicitly restricted to variable families to align with +ASC-corrected alignment populations. Additionally, fixed `--exclude-genome` handling to re-scan surviving sequences and drop newly monomorphic columns, preventing silent data corruption in downstream tree inference tools.
This commit is contained in:
@@ -1094,15 +1094,136 @@ identified. Gives 120 parameters, but derived from two small,
|
||||
well-estimated pieces (a 5×5 cardinality model, a 4×4 composition model)
|
||||
rather than fit or smoothed independently per pair.
|
||||
|
||||
**Status: designed, not implemented.** Would replace `build_cost_matrix`
|
||||
(`obikindex/src/sankoff.rs`) and the single `c_ctx` scalar/parameter
|
||||
entirely; `sub_cost`'s own calibration is untouched. Not yet decided
|
||||
whether the "clean" parsimony-graph cost (`build_cost_matrix`'s current
|
||||
output, still needed for `--tnt`/`--phyg`) should be replaced too, kept
|
||||
as a separate simpler approximation, or derived as a special case of this
|
||||
same decomposition (it likely can be — the same `shared`/`lost`/`gained`
|
||||
logic, with the cardinality model reduced back to a single scalar `c_ctx`
|
||||
substituted in, recovers exactly the current graph).
|
||||
### Implemented (2026-08-12): `pairwise_cost_matrix` replaces `build_cost_matrix` entirely
|
||||
|
||||
New module `obikindex/src/cardcomp.rs`, replacing `sankoff::build_cost_matrix`
|
||||
and the `c_ctx`/`SankoffWeights`/`PHatEstimate`/`calibrate_p_hat`/
|
||||
`c_ctx_from_p_hat`/`mean_substitution_cost`/`substitution_costs_from_tally`
|
||||
machinery it depended on outright — not kept in parallel as a fallback
|
||||
(all now unreferenced outside their own tests; `sankoff.rs` itself is a
|
||||
pending removal, not yet done).
|
||||
|
||||
**New primitives, `obikindex/src/siblings.rs`:**
|
||||
- `BasePairTally` gained a `same: [u64; 4]` field (diagonal — "both
|
||||
genomes at the same single base", pooled from
|
||||
[`base_pair_tally`](obikindex::KmerIndex::base_pair_tally)'s existing
|
||||
traversal, extended to also tally the `bi == bj` case it previously
|
||||
discarded).
|
||||
- `CardinalityTally { counts: [[u64; 5]; 5] }` and
|
||||
`KmerIndex::cardinality_tally`, a new traversal (same shape as
|
||||
`snp_pseudo_alignment`'s — needs full per-genome presence masks, not
|
||||
`scan_family_pairs`'s single-resolved-form view, since cardinality 2-4
|
||||
is exactly the signal being tallied, not noise to drop). Same
|
||||
saturated/no-data pair exclusion as `base_pair_tally`. Restricted to
|
||||
variable families (`family_size() >= 2`), matching
|
||||
`snp_pseudo_alignment`'s own scope. Verified against the 20-genome
|
||||
benchmark: counts match the earlier hand-rolled Python analysis exactly
|
||||
(e.g. `c=0/c=0: 117,158,166`, `c=0/c=1: 13,707,223` — the same numbers
|
||||
this whole investigation started from).
|
||||
|
||||
**`cardcomp.rs`:**
|
||||
- `cardinality_transition_probs`/`composition_transition_probs`: row-
|
||||
normalise the two tallies into proper transition probability matrices,
|
||||
diagonal included ("stay the same" is a real, calibrated outcome).
|
||||
- `pairwise_cost_matrix`: for every pair of the 16 states, `shared = A∩B`
|
||||
contributes `∏ P_composition(x→x)`; `lost = A\B`, `gained = B\A` are
|
||||
parsimony-paired (`best_pairing_cost`, brute-force over the ≤4!
|
||||
injections — small enough that hand-rolling beats a dependency) into
|
||||
substitution events on `P_composition`, minimising total `-ln`; the
|
||||
cardinality-difference leftover is priced once via
|
||||
`P_cardinality(|A|→|B|)`, never chained. Row-normalised, `-ln`'d, then
|
||||
**symmetrised**: `cost_sym(A,B) = (cost(A,B)+cost(B,A))/2` — equivalent
|
||||
to taking the *geometric* mean of the two raw probabilities
|
||||
(`-ln(√(P(A,B)·P(B,A))) = (-ln P(A,B) - ln P(B,A))/2`), not their
|
||||
arithmetic mean. Required, not just convenient for IQ-TREE's
|
||||
lower-triangular file format: Sankoff parsimony's score is independent
|
||||
of where an *unrooted* tree (what TNT/PhyG actually search over) gets
|
||||
rooted only if the cost matrix is symmetric — the discrete-parsimony
|
||||
analogue of CTMC reversibility, established by direct reasoning, not
|
||||
assumed. Bonus of the same decision: 120 free parameters instead of the
|
||||
240 a fully asymmetric matrix would need.
|
||||
|
||||
**Verified on the 20-genome benchmark**: resulting matrix symmetric
|
||||
(checked numerically, zero asymmetric cells), zero diagonal, no NaN/Inf.
|
||||
`--tnt` output still loads into TNT with no triangle-inequality warning
|
||||
(`scaled_metric_matrix`'s rounding-metric-closure step still needed and
|
||||
still applied — nothing in the new construction guarantees the *rounded
|
||||
integer* matrix stays a metric, even though the real-valued one is exact
|
||||
by construction here, unlike the old Floyd-Warshall-closed matrix which
|
||||
needed it for a different reason). IQ-TREE loads the new model file and
|
||||
reports the same `π` as before (only `R` changed).
|
||||
|
||||
### Two consistency bugs found and fixed post-implementation (2026-08-13)
|
||||
|
||||
**`--exclude-genome` didn't drop columns that become monomorphic once the
|
||||
excluded genome(s) are gone.** `snp_pseudo_alignment`'s "variable family"
|
||||
test (`family_size() >= 2`) is a property of the annex computed over
|
||||
*every* genome in the index — unaffected by the CLI-level exclusion, which
|
||||
only dropped the excluded genome's *row*. A family variable only because
|
||||
of the excluded genome stayed in the alignment as a now-constant column —
|
||||
silently wrong data for TNT/PhyG, and a hard failure for IQ-TREE's `+ASC`
|
||||
(verified: excluding 2 taxa on the benchmark left 116,351 such columns —
|
||||
matches the manual `+ASC` failures hit earlier in this same investigation,
|
||||
before `--exclude-genome` existed). Fixed in `drop_excluded`
|
||||
(`obikmer/src/cmd/distance/mod.rs`): after dropping excluded rows,
|
||||
re-scan each column among the *surviving* sequences and drop any that are
|
||||
now constant. Verified: 908,723 → 792,372 sites after excluding 2 taxa,
|
||||
zero monomorphic columns remain, `π` recomputed from the corrected
|
||||
alignment matches an independent recount exactly. The compact-alphabet
|
||||
renumbering (`iqtree::compact_alphabet`) needed no equivalent fix — it
|
||||
already recomputes which of the 16 states occur fresh on every call, from
|
||||
whatever alignment it's actually handed, so a symbol disappearing (e.g.
|
||||
excluding every genome that carries `N`) is already handled correctly;
|
||||
verified directly (excluded 13 genomes to force `N` out: "15 of 16 states"
|
||||
reported, correctly-shaped model file).
|
||||
|
||||
**`cardinality_tally`'s `family_size() >= 2` filter looked inconsistent
|
||||
with `base_pair_tally` — removing it was tried, and was wrong; reverted.**
|
||||
`cardinality_tally` (modelled after `snp_pseudo_alignment`) had the
|
||||
filter; `base_pair_tally` didn't (it visits every family via
|
||||
`scan_family_pairs` unconditionally, folding fully-invariant loci into its
|
||||
own `same` diagonal). Read as `cardinality_tally` under-counting its
|
||||
diagonal relative to `base_pair_tally`, and — independently — as another
|
||||
angle on the `--exclude-genome` drift (`family_size()` being global-only
|
||||
meant a family kept here post-exclusion could differ from what the
|
||||
now-correctly-filtered alignment kept). First fix tried: drop
|
||||
`cardinality_tally`'s filter entirely, matching `base_pair_tally`'s
|
||||
whole-annex scope.
|
||||
|
||||
**That fix was empirically wrong, confirmed by a real IQ-TREE run, not
|
||||
just a hunch.** Log-likelihood dropped from the earlier correct run's
|
||||
`-8,364,671`/`-8,371,082` to `-9,170,228` (worse fit, not better), with
|
||||
repeated `NNI search needs unusual large number of steps (20) to
|
||||
converge!` warnings — and the completed run's **total tree length came
|
||||
out at 67.644**, roughly 30× the earlier correct runs' ~2.0, i.e. branches
|
||||
blowing up/saturating. Root cause, only clear in hindsight: `+ASC`
|
||||
("ascertainment bias correction") exists specifically because the
|
||||
likelihood only ever sees *variable* sites — the alignment fed to
|
||||
IQ-TREE, by construction, contains not one invariant column. Calibrating
|
||||
`R` from a population overwhelmingly dominated by genome-wide invariant
|
||||
background (family_size()<2 loci outnumber the ~908k variable ones by
|
||||
orders of magnitude) describes a completely different population than the
|
||||
one `+ASC` and the alignment actually model — the "consistency" argument
|
||||
for matching `base_pair_tally`'s scope was real, but pointed the wrong
|
||||
way: `base_pair_tally`'s own unrestricted `same` diagonal turned out to
|
||||
have the *identical* latent bug (only unmasked once its diagonal existed
|
||||
at all, which happened earlier the same day when `same` was added), not a
|
||||
correct baseline to match `cardinality_tally` to.
|
||||
|
||||
**Final fix**: restored `cardinality_tally`'s `family_size() >= 2` filter,
|
||||
and gave `base_pair_tally`'s `same` diagonal the equivalent restriction —
|
||||
`scan_family_pairs` (shared with `raw_snp_distance`, which legitimately
|
||||
*does* want fully-invariant families counted as `shared`) now passes an
|
||||
extra `variable: bool` (the family's own `family_size() >= 2`) to its
|
||||
`on_pair` callback; `base_pair_tally` only increments `same` when
|
||||
`variable` is true, `raw_snp_distance`'s callback ignores the new
|
||||
argument. Both tallies now describe the same variable-families-only
|
||||
population the `+ASC`-corrected alignment does. Verified: calibration
|
||||
counts back to their original values exactly (`c=0/c=0`: 117,158,166,
|
||||
matching the pre-regression run bit for bit), and a full IQ-TREE rerun
|
||||
converged normally — log-likelihood `-8,389,106.273` (same order as the
|
||||
two earlier correct runs), **total tree length 2.044** (was 67.644), no
|
||||
NNI convergence warnings.
|
||||
|
||||
## Heterozygosity, ploidy, and consensus-assembly inputs
|
||||
|
||||
|
||||
Reference in New Issue
Block a user