Simulate ancestral alignments under the TKF and substitutions models by MattesMrzik · Pull Request #140 · acg-team/phyne

MattesMrzik · 2026-01-23T17:11:04Z

This PR implements an AlignmentSimulation trait which simulates an ancestral alignment and implements it for the TKF91 and TKF92 models and substitution models. It also introduces a utility for removing non surviving homology paths and ancestral alignment transformation into standard leaf-only alignments.
Key Changes

TKF Indel MSA Simulation
Full Evolutionary Simulation: Added TKFMSASimulator which combines the indel process with a substitution model to produce complete, character-based alignments.
Substitution Simulator: Introduced a standalone SubstitutionSimulator for generating alignments with substitutions only.
Ancestral Alignment Enhancements:
- Added remove_extinct_columns to clean up alignments where ancestral characters do not survive to any leaves.
- Implemented into_alignment to transform ancestral alignments (containing internal nodes) into standard alignments.
Test: Added tests covering mutation constraints, Dollo’s constraint compliance, and fragment capping logic.

codecov · 2026-01-23T18:48:21Z

Codecov Report

❌ Patch coverage is 96.42445% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.76%. Comparing base (527040d) to head (abe7545).
⚠️ Report is 41 commits behind head on develop.

Files with missing lines	Patch %	Lines
phyne/src/phylo_info/mod.rs	94.78%	6 Missing ⚠️
phyne/src/alignment/mod.rs	90.90%	5 Missing ⚠️
phyne/src/tkf_model/tkf_indel.rs	28.57%	5 Missing ⚠️
phyne/src/tkf_model/simulate_msa/fragmentation.rs	93.84%	4 Missing ⚠️
...ne/src/tkf_model/simulate_msa/sim_tkf_indel_msa.rs	98.95%	4 Missing ⚠️
phyne/src/tkf_model/simulate_msa/sim_tkf_msa.rs	94.52%	4 Missing ⚠️
phyne/src/substitution_models/simulate_msa.rs	97.47%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #140      +/-   ##
===========================================
+ Coverage    96.32%   96.76%   +0.43%     
===========================================
  Files           32       52      +20     
  Lines         4355     6486    +2131     
===========================================
+ Hits          4195     6276    +2081     
- Misses         160      210      +50

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Replace local WILDCARD_CHAR constant (b'N') with the shared AMB_CHAR from crate::alphabets for consistency - Move TKF92MSASimulationResult struct definition after the link/fate enums so related types are grouped more logically - Add clarifying doc comments on Seqs type alias and the indel_model field

- Remove the local tkf92_fixed test helper and naive_merge utility, which duplicated indel cost logic already encapsulated in TKF92FixedIndelCostBuilder - Update the simulate test to build and invoke the cost via TKF92FixedIndelCostBuilder, keeping the assertion equivalent - Clean up unused imports (AncestralAlignment, h1, log_i1, log_n1, get_mapping_for_any_node)

- Add FragmentSampler trait with sample_fragment_length<R: Rng> -> (usize, f64), returning the sampled length and its log-probability - Implement FragmentSampler for TKF91IndelModel (always returns length 1, log-prob 0.0) - Implement FragmentSampler for TKF92IndelModel (geometric draw using r parameter) - Rename TKF92MSASimulator -> TKFMSASimulator<T: TKFModel + FragmentSampler, Q, R>, replacing the hardcoded TKF92IndelModel field with the generic T - Delegate sample_fragment_length on the simulator to the trait, eliminating all model-specific logic from the simulator itself - Rename simulate test to tkf92_simulate; add tkf91_simulate test that asserts every fragment has length 1 and logl matches TKF91IndelCostBuilder

- Replace root_residue_count accumulator with fragmentation.last().unwrap_or(&0) + current_link.length - Drop the root_residue_count: usize parameter from append_link_to_msa and links_to_msa - Boundary computation is now self-contained: each entry is the previous boundary plus the current fragment's length, which is correct because fragmentation is a monotonically increasing cumulative sequence

- Extract dispatch_immortal_children to handle immortal link traversal - Extract process_link to handle non-immortal link MSA writes - Extract apply_fate to apply a single branch fate (homolog/deletion/non-homolog) - Add explicit 'a lifetime annotations to process_link, apply_fate and dispatch_immortal_children so that references pushed onto tree_stack and insertions are tied to the lifetime of the source TKFLink data

- Rename TKF92MSASimulationResult to TKFMSASimulationResult<AA> - Make simulate_msa() return typed struct instead of tuple - Add msa_to_alignment_with_non_emitting_cols() method - Fix typos: didnt→didn't, comulative→cumulative, bc→because - Rename test tkf_homlog_probs to tkf_homolog_probs

- Move trait bounds into a clause for readability in

- it seems to be simpler to write simulation logl match in tests only

- Add SubstitutionSimulator and SubstitutionSimulatorBuilder in src/substitution_models/simulate.rs - Precompute transition matrices P(edge) once and reuse for all sites - Fixed alignment length provided via builder (set at construction) - Root sampled from model equilibrium freqs; traverse preorder to sample children - Export new module in src/substitution_models/mod.rs - Add unit tests for correctness and reproducibility

Replace EvoModel with SubstModel<Q> in substitution simulator - Make builder and simulator generic over Q: QMatrix and store SubstModel<Q> - Reimplement p(time) locally when precomputing P = exp(Q * t) to avoid EvoModel dependency - Access frequencies and alphabet via the underlying QMatrix - Update tests to construct SubstModel<JC69> (no API changes to tests) Files changed: - src/substitution_models/simulate.rs Rationale: keeps simulator tied to concrete substitution model representation and avoids using the dynamic EvoModel trait where unnecessary. This simplifies access to Q and its frequencies while preserving behavior and reproducibility.

Refactor generics in substitution simulator to use where clauses - Move generic bounds into clauses for builder, simulator and impl blocks - Improves readability by reducing line length - No functional changes File changed: - src/substitution_models/simulate.rs

- Replace JC69 with GTR in substitution simulator private tests - Provide concrete GTR frequencies and rate parameters for deterministic behavior - Keep existing tree! macro usage unchanged This makes tests use a realistic, parameterized DNA model rather than JC69, improving test coverage for general substitution models.

- Rename substitution simulator module file to - Update module export in to use - Preserve tests and functionality while switching filename to better reflect purpose

Add default method to trait and support methods in MASA. - Detect columns where all leaf maps are and remove them - Update leaf and ancestral mappings and sequences accordingly - Add helpers on MASA to efficiently update internal state - Note: update deferred (see issue acg-team#150)

- Add complex test `into_alignment_masa_to_msa` in `src/alignment/tests.rs

junniest · 2026-09-23T16:00:06Z

+}
+
+/// Rebuilds a clean tree from a (possibly mutated) tree, dropping any nodes that are no
+/// longer connected to the root. Serialization to Newick and re-parsing re-computes all


Uhhhh, I understand why this is the easiest way to handle tree creation in this scenario, but this seems wrong somehow... using serialisation as a tool for anything other than output.

Would there be an easy way to create the tree without resorting to newick that we could add?

Lets discuss in person if we even want the prune_empty_leaves method (its the only place where this rebuild_tree is used.

If I remember correctly we decided to keep it for now.

junniest · 2026-09-23T16:04:06Z

+
+    /// Returns all sequences (leaf and ancestral) in aligned form, mirroring
+    /// [`Display`](`std::fmt::Display`) for MASA. Node ids are resolved through the tree,
+    /// since `idx_to_id` is private to the alignment module.


This also somehow feels wrong that you have to resolve node ids manually. Could we implement a helper that isn't relying on private indices, or work around this a different way?

MattesMrzik · 2026-09-24T12:22:28Z

(An alternative would be to have this code as a separate crate that uses the lib via its API)

- removed into_alignment, its now discard ancestral seqs in phyloinfo - use our rng in fragment sampler - keep col mask is now bitverctor - sim subst msa split into multiple methods

junniest · 2026-10-14T14:00:20Z

+
+        let aligned = self.aligned_seqs();
+        let pruned_tree = prune_empty_leaves_from_tree(&self.tree, &leaves_to_remove)
+            .unwrap_or_else(|e| {


wouldn't it make sense to call it with a ? instead, since prune_empty_leaves_from_tree also returns a result?

changed the method to panic on bug.

junniest · 2026-10-14T14:22:55Z

+}
+
+impl TKFLink {
+    fn new_immortal(node: NodeIdx) -> Self {


could TKFLink be an enum type instead? So that instead of checking if link.is_immortal you could do a match?

I think the refactor is not worth the hustle.

junniest

Only one comment (‎phyne/src/alignment/mod.rs‎ Line 140) is really a potential issue here, the rest are nitpicks in reality.

Good job!

MattesMrzik added 21 commits March 5, 2026 09:00

simulate msa, logl == calculation without aggregation

623e64d

cap max insert len are not bail anymore, cleanup

b5f8663

Add unit tests for TKF92 probability calculations

fcd2bbe

fragments without helper char

7acd527

Use where clause on impl header

96891d9

- Move trait bounds into a clause for readability in

removed old check simulation logl method

b56753f

- it seems to be simpler to write simulation logl match in tests only

simulation trait; removed unimpled Q from tkf sim

6a67ffb

improved skip root;

10f6c8b

rename simulate to simulate_msa

3424afd

- Rename substitution simulator module file to - Update module export in to use - Preserve tests and functionality while switching filename to better reflect purpose

impled indel maskinf for subst, still diagnostics

9db2516

fixed errors

1e31b3a

MattesMrzik force-pushed the simulate-tkf92-msa branch from d4b2a1d to 1e31b3a Compare March 6, 2026 13:33

MattesMrzik added 6 commits March 18, 2026 12:37

used correct tree macro in tests

3c7d52a

actually remove cols; test; warn in aa creation

d3a48c9

default impl to AA to A

3e7a7f4

tests: MASA->MSA conversion & mappings

17e4c1c

- Add complex test `into_alignment_masa_to_msa` in `src/alignment/tests.rs

added comment about empty col after reestimation

af9f55e

MattesMrzik marked this pull request as ready for review March 19, 2026 14:28

MattesMrzik requested a review from junniest September 19, 2026 11:21

junniest reviewed Sep 23, 2026

View reviewed changes

This was referenced Sep 28, 2026

Refactor Alignment #190

Open

Internal representation of a fasta record to allow mutability of its fields #143

Open

sim returns phylo; prune empty leaf in phylo

35b1e36

- removed into_alignment, its now discard ancestral seqs in phyloinfo - use our rng in fragment sampler - keep col mask is now bitverctor - sim subst msa split into multiple methods

junniest reviewed Oct 14, 2026

View reviewed changes

MattesMrzik added 15 commits October 15, 2026 14:34

bug! macro; smaller requested changes

9f588ff

Merge remote-tracking branch 'origin/develop' into simulate-tkf92-msa

5f352f7

fix after merge

1c80893

del old phylo dir

d77fd36

max ins frag len as option

3657e70

fix tautological test

5955631

fix link in doc

40785e3

fix max ins len overflow

b1cc746

test: remove extinct cols extreme cases

53cb184

added test for sim msa trait

6c71bc7

test:remove extinct of sim res; prune leaf doctest

bef6380

tests for coverage

f32ac2c

more doc test

40dedd9

docs use AA instead of MASA

01cdc80

more docs polish

ed17f03

junniest approved these changes Oct 16, 2026

View reviewed changes

MattesMrzik added 6 commits October 19, 2026 15:13

refactor tkf link

ded8775

fix AA from aligned extinct seqs warn

424210b

implemented suggestions

5720555

fix doc tests

df6ee1a

Bug prefix; spelling

864b82b

fix test

abe7545

MattesMrzik merged commit 71d7ce2 into acg-team:develop Oct 20, 2026
9 checks passed

Conversation

MattesMrzik commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattesMrzik commented Sep 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junniest left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MattesMrzik commented Jan 23, 2026 •

edited

Loading

codecov Bot commented Jan 23, 2026 •

edited

Loading

junniest left a comment •

edited

Loading