Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions kb/decisions/optimize-polytomy-reversion-resolution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Optimize loop resolves reversion-driven polytomies

The `optimize` loop adds a topology move with no v0 counterpart: when a polytomy forces a substitution onto an internal edge and then reverts it on a child, the move relocates the child so the reversion cancels. This removes homoplasy that an arbitrary binary resolution of a polytomy introduces, and that v0's loop never targets.

## What v0 does

v0's branch-length optimization loop (`TreeAnc.optimize_tree_marginal`) collapses short internal branches (`prune_short_branches`) but has no step that inspects or removes reversions. A reversion is only ever dropped as an incidental side effect when an edge is collapsed for an unrelated reason and its substitutions compose with the child's. v0 has no shared-mutation merge and no reversion-aware polytomy handling.

## What v1 does

Once per iteration, [`prune_and_merge_in_loop()`](../../packages/treetime/src/optimize/run_loop.rs#L319) runs a single per-polytomy routine, [`resolve_polytomies()`](../../packages/treetime/src/optimize/topology/resolve_polytomy.rs#L31), after the zero-optimal edge collapse. Sparse partitions only; dense partitions carry no per-edge mutation lists. The routine applies three steps at each polytomy and iterates to a fixpoint:

- Merge siblings that share substitutions under a helper node ([`merge_single_polytomy()`](../../packages/treetime/src/optimize/topology/merge_shared_mutations.rs#L64)). This canonicalizes several children reverting the same position into one reverting child.
- Hoist the reverting child with the largest reversion count ([`hoist_reverting_child()`](../../packages/treetime/src/optimize/topology/hoist_reversions.rs#L74)), ties broken by edge key.
- Retire helper nodes: collapse mutation-free edges to nodes the routine created in this pass.

The hoist inserts a new node $N$ between parent $u$ and node $v$, grouping $v$ with one reverting child $c$. The parent-edge substitutions $M_v$ partition by position against the child substitutions $M_c$ into three disjoint sets: $T$ (untouched by the child), $H$ (chained, $a \to b$ then $b \to d$), and $R$ (reverted, $a \to b$ then $b \to a$). The resulting edges carry:

| edge | substitutions |
| --------- | ------------------------------------------------------------------------------------------------------ |
| $u \to N$ | $T$ |
| $N \to v$ | $H \cup R$ |
| $N \to c$ | $\mathrm{compose}(M_v, M_c)$ at $M_c$ positions ($H' \cup D$, where $D$ are the child's own positions) |

The net substitution change is $\Delta = -\lvert R\rvert$: one mutation removed per reverted position, none added. The two relocated edges keep their keys via [`Graph::reparent_edge()`](../../packages/treetime-graph/src/graph_ops.rs#L123), so their partition entries stay valid and only their content is rewritten.

Branch lengths split in proportion to substitution count so that root-to-$v$ and root-to-$c$ distances are preserved exactly and re-fit on the next iteration: $b(u \to N) = b(u \to v)\cdot\lvert T\rvert / \lvert M_v\rvert$, $b(N \to v) = b(u \to v) - b(u \to N)$, $b(N \to c) = b(N \to v) + b(v \to c)$. This deliberately differs from the Jukes-Cantor recomputation used by the `prune` merge ([prune-merge-jukes-cantor-branch-length.md](prune-merge-jukes-cantor-branch-length.md)), which would discard the converged branch lengths inside the loop.

Indels use an all-or-nothing rule, because `compose_indels` merges overlapping and adjacent ranges rather than being position-keyed. When no child indel overlaps or is adjacent to a parent indel, the parent indels move cleanly above $N$ and $N \to c$ carries the child's own indels; otherwise the parent indels stay on $N \to v$ and $N \to c$ carries the full composition. Both branches preserve the root-to-$v$ and root-to-$c$ indel content, and the substitution gain is unaffected.

## Why v1 differs

An arbitrary binary resolution of a polytomy can spend two mutations (a substitution plus its reversion) where one suffices, and tree builders produce such resolutions routinely. Fitch's forward pass never emits a reversion on a binary node's child edge, so the pattern is inherently a polytomy phenomenon; that is why the move belongs in the per-polytomy routine rather than as a general edge-level rewrite. Inserting a node beats re-attaching the child directly to the parent, which would duplicate $T$ onto the re-attached edge and give $\Delta = \lvert R\rvert - \lvert T\rvert$, almost never a gain because $\lvert T\rvert$ is typically much larger than $\lvert R\rvert$.

The routine runs every iteration, independent of whether a collapse fired, so reversions present in the input and in polytomies formed by earlier iterations are resolved. It cannot oscillate with the zero-optimal collapse: the lexicographic (total fitch mutation count, node count) potential strictly decreases on every applied merge, hoist, or retirement, even though the hoist deliberately relocates mutation-carrying edges that the collapse step refuses to touch.

## Greedy limitation

When every child of a polytomy reverts the same position, the merge reduces them to a single reverting child and the hoist declines (moving that child would leave the node childless). One residual reversion remains rather than dissolving a pre-existing node ([kb/issues/M-optimize-reversion-hoist-single-child-residual.md](../issues/M-optimize-reversion-hoist-single-child-residual.md)). Disagreeing reverting children likewise leave irreducible homoplasy; exact resolution is subtree parsimony re-optimization, which is out of scope.

## Affected commands

- [`optimize`](../../packages/treetime/src/optimize/run_loop.rs#L319) - runs the routine in its per-iteration topology-cleanup step
- `prune` is unchanged: it still calls the shared-mutation merge directly and does not adopt the reversion hoist

## Tests

- Unit (hoist mechanics): [test_hoist_reversions.rs](../../packages/treetime/src/optimize/topology/__tests__/test_hoist_reversions.rs) - no duplication of $T$, chained composition, reversion removal, distance preservation, per-partition splits, indel cases.
- Integration (merge to hoist to retire): [test_resolve_polytomy.rs](../../packages/treetime/src/optimize/topology/__tests__/test_resolve_polytomy.rs) - the worked example reaching the parsimony optimum, incompatible splits stopping at the greedy bound, helper retirement sparing pre-existing internal nodes, root and reversion-free polytomies left untouched.
- Property: [test_prop_resolve_polytomy.rs](../../packages/treetime/src/optimize/topology/__tests__/test_prop_resolve_polytomy.rs) - the potential never rises and strictly falls on any change, and leaves, non-negative branch lengths, and the single-root tree shape are preserved.
11 changes: 11 additions & 0 deletions kb/features/optimize.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ v0 uses Brent's method (`scipy.optimize.minimize_scalar`) in sqrt(t) space with
- [ ] Bifurcating root special handling (v0 optimizes combined root-children length, preserves ratio)
- [ ] Convergence by sequence change count (v0 joint mode: stops when zero nucleotides change)

## Topology Cleanup (v1-only)

Once per iteration, `prune_and_merge_in_loop` simplifies the tree. Sparse partitions only; dense partitions carry no per-edge mutation lists, so the reversion and merge steps are inert under a dense-only run.

- [x] Zero-optimal edge collapse: internal edges the per-edge optimizer drove to exactly zero, carrying no substitutions or indels, are contracted into polytomies. Mirrors v0's `prune_short_branches` inside the loop.
- [x] Shared-mutation merge: siblings in a polytomy that carry identical substitutions are grouped under a new internal node (`merge_shared_mutation_branches`).
- [x] Reversion hoist: when a child edge reverts a substitution on the node's parent edge, a new node is inserted grouping that child with its sibling subtree, lifting the non-reverted substitutions above it. Removes one mutation per reverted position and adds none; branch lengths split proportionally to preserve root-to-node distances ([kb/decisions/optimize-polytomy-reversion-resolution.md](../decisions/optimize-polytomy-reversion-resolution.md)).
- [x] Helper-node retirement: mutation-free edges to nodes created during the routine are collapsed, dissolving the transient helpers the merge and hoist leave behind.

The merge, hoist, and retire steps form one per-polytomy routine (`resolve_polytomies`) run every iteration and driven to a fixpoint by a monotone (mutation count, node count) potential. A polytomy whose children all revert the same position collapses to a single reverting child that the two-children guard leaves in place ([kb/issues/M-optimize-reversion-hoist-single-child-residual.md](../issues/M-optimize-reversion-hoist-single-child-residual.md)).

## GTR Integration

- [x] `--model` flag wired through `get_gtr_sparse()`/`get_gtr_dense()` for all named models and inference
Expand Down
28 changes: 28 additions & 0 deletions kb/issues/M-optimize-reversion-hoist-single-child-residual.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Reversion hoist leaves a residual reversion when every child reverts the same position

## Symptom and reproduction

In the optimize loop, `resolve_polytomies` resolves a reversion polytomy by merging siblings that share substitutions, then hoisting the reverting group under a new node. When _every_ child of a polytomy node reverts the same parent-edge substitution, the shared-mutation merge groups all of them under one helper node, leaving the polytomy node with a single child. The hoist then declines to fire, because moving that single reverting child under a new node would leave the polytomy node childless (a spurious leaf).

The result keeps one residual reversion on the merged child edge. The total mutation count still drops relative to the input (the merge removes the shared reversion from every duplicate), but it does not reach the parsimony optimum, which would remove the reversion entirely.

Reproduction: a node `V` under parent edge `U -> V = {A0C}` whose children all carry `C0A`. After the merge, `V` has a single child (the helper) carrying `C0A`; the optimum is zero mutations (drop `A0C` and the reversion), but the routine stops at two (`A0C` on `U -> V`, `C0A` on the helper edge).

## Impact and scope

Narrow. It affects only polytomies where all children revert the same position, and only leaves a reversion that the merge already reduced from many to one. The output is never worse than the input and never invalid. Correctness (no added mutations, distance preservation, single-rooted tree) is unaffected.

## Root cause

`fn try_hoist_reverting_child()` requires the node to keep at least two children so the hoisted child leaves a sibling behind. A single-child node drops below that threshold. The optimal resolution here is to collapse the now-degree-two node into its parent (composing the two edges cancels the reversion), but collapsing a pre-existing node contradicts the routine's helper-retirement scope, which deliberately never dissolves nodes the input asserted.

## Fix approach

Recognize a degree-two node whose single child reverts its parent edge and collapse that node's parent edge (compose `M_v` with `M_c`, cancelling the reversion). This removes a topologically vacuous node (a single-child node carries no phylogenetic split), so it preserves all bipartitions. Requires distinguishing this case from the helper-retirement scope, and a decision on whether removing pre-existing degree-two nodes during `optimize` is acceptable.

This is a greedy limitation in the same family as the incompatible-splits case the routine already accepts (disagreeing reverting children leave irreducible homoplasy); exact resolution is subtree parsimony re-optimization, which is out of scope.

## Locations

- `fn try_hoist_reverting_child()` two-children guard: `packages/treetime/src/optimize/topology/resolve_polytomy.rs`
- Hoist move: `packages/treetime/src/optimize/topology/hoist_reversions.rs`
Loading
Loading