Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend path algorithm (extend_edgesV2) #2938

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hfr1tz3
Copy link

@hfr1tz3 hfr1tz3 commented Apr 25, 2024

New extension (pun intended) to the extend_edges algorithm now called extend_paths (subject to change). We noticed that certain examples within unary regions would not be extended simply with extend edges. We propose this algorithm to handle examples like the following:
For edge path $a \to b \to c$ in tree $T_1$ and edge $a \to d \to c$ in $T_2$, assuming $d\notin T_1$ and $b\notin T_2$ and $t(b)>t(d)$ we should have extended path $a\to b \to d \to c$ in tree $T_2$.
Unit tests still need to be built, and the algorithm needs to be cleaned.

We hypothesize that using this alongside extend edges could be the optimal strategy for minimizing edges in tree sequences.

Copy link

codecov bot commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.65%. Comparing base (e7fdbe1) to head (ca1a5f4).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2938      +/-   ##
==========================================
- Coverage   89.63%   86.65%   -2.98%     
==========================================
  Files          29       11      -18     
  Lines       30174    22932    -7242     
  Branches     5873     4254    -1619     
==========================================
- Hits        27046    19872    -7174     
+ Misses       1789     1754      -35     
+ Partials     1339     1306      -33     
Flag Coverage Δ
c-tests 86.20% <ø> (ø)
lwt-tests 80.78% <ø> (ø)
python-c-tests 88.72% <ø> (ø)
python-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 18 files with indirect coverage changes

@petrelharp
Copy link
Contributor

petrelharp commented May 15, 2024

Proposal for the basis for a test: here is a definition of what should be extendable by this method; so we can check if there remain no extendable edges (when run until no further changes are made).

A right extendable path from $x$ to $y$ in a tree $T_k$ is a path $x &lt;_k a_1 &lt;_k \cdots &lt;_k a_n &lt;_k y$ (where $&lt;_k$ denotes "below in tree $T_k$" that has the properties:

  1. $x <{k+1} b_1 <{k+1} \cdots <{k+1} b_m <{k+1} y$ for some $b_i$ (the endpoints of the path are in the next tree)
  2. for $1 &lt; j &lt; n$, $a_j$ is not in $T_{k+1}$, and for $1 &lt; j &lt; m$, $b_j$ is not in $T_{k}$ (none of the intermediate nodes from the two paths are in the other tree).

The nodes $a_i$ are right extendable, and the nodes $b_i$ are left extendable.

Why is this the right definition? Well, first it's clear we can't extend nodes that are in the next tree. Next, suppose that there is a $b_j$ on the path from $x$ to $y$ in $T_{k+1}$ that is also in $T_k$, but not on the path from $x$ to $y$. If $b_j &lt;_k x$ then we can take $b_j$ as the other endpoint of the extendable path instead of $y$. If $b_j \nless_k x$, then that implies ancestry above $b_j$ was inherited along a different path back to the time of $x$, so we can't use inheritance from $x$ as a criteria for extending. A similar argument holds if $y &lt;_k b_j$ or and $y \nless_k b_j$.

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.

None yet

2 participants