fix: hoist reversions across a bifurcating root - #950
Merged
ivan-aksamentov merged 3 commits intoSep 3, 2026
Conversation
- A degree-2 root is a reversible pass-through, so the substitution a child reverts can sit on the sibling edge, leaving the node's own parent edge empty at that site and hiding the reversion from per-node detection - Score children against the parent edge augmented with the inverted sibling substitutions, then re-root the winning positions onto the parent edge so the existing hoist removes the reversion, making the move invariant to bifurcating-root placement - Move the root clamp (root_sequence and the root node MAP) in lock-step so the next iteration's marginal reconstruction stays consistent with the rewritten edges
- Unit tests for the root slide: it moves the sibling substitution onto the parent edge and the root clamp, and the following hoist removes the exposed reversion - Integration test on a bifurcating-root worked example reaching the one-mutation bipartition optimum - Property test asserting the site cost is the reroot-invariant bipartition cost regardless of how many children carry the substitution, with the potential still falling on every change
- Document the bifurcating-root case in the reversion-resolution decision: the sibling edge can hold the substitution a child reverts, and the root slide re-roots the site so the standard hoist removes it, making the move invariant to bifurcating-root placement - Update the test list to cite the new slide, worked-example, and reroot-invariant property coverage
ivan-aksamentov
deleted the
fix/optimize-root-hoisting-bifurcating-root
branch
September 3, 2026 23:51
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
1 participant
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.
Update:
Problem: the reversion hoist reads only a node's own parent edge. At a bifurcating (degree-2) root, the two child edges are two halves of one unrooted edge, so a substitution a child reverts can lie on the sibling edge rather than the parent edge, and the per-node scan misses it. The tree keeps two mutations instead of one, and the count depends on root placement.
Fix: score each child against the parent edge combined with the inverted sibling substitutions, then move the root state at the reverted sites onto the sibling before the hoist removes the reversion. Nodes not directly under a bifurcating root are unchanged.
Correctness: the re-rooting preserves the total mutation count and runs only for sites the following hoist removes, so the potential strictly decreases on each change.
Result: the mutation count no longer depends on bifurcating-root placement, and equals the count obtained when the reversion lies on an internal edge. Applied only to degree-2 roots.
Work items
Screenshots
optimizeon the reported tree, rendered in auspice.us.Before fix (9 mutations). Node
Ashows one change (C3T) and two reversions to root at positions 6 and 7.After fix (7 mutations). The shared substitutions moved to the ancestral branch and node
Ashows no sequence changes.