Skip to content

Use virtual root #1799

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 20, 2021
Merged

Use virtual root #1799

merged 2 commits into from
Oct 20, 2021

Conversation

jeromekelleher
Copy link
Member

Stacked on #1704

@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #1799 (02d4771) into main (e622480) will decrease coverage by 0.02%.
The diff coverage is 91.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1799      +/-   ##
==========================================
- Coverage   93.40%   93.38%   -0.03%     
==========================================
  Files          27       27              
  Lines       24773    24783      +10     
  Branches     1095     1094       -1     
==========================================
+ Hits        23140    23144       +4     
- Misses       1598     1604       +6     
  Partials       35       35              
Flag Coverage Δ
c-tests 92.11% <93.33%> (-0.04%) ⬇️
lwt-tests 89.14% <ø> (ø)
python-c-tests 94.53% <85.71%> (-0.01%) ⬇️
python-tests 98.75% <100.00%> (ø)

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

Impacted Files Coverage Δ
python/_tskitmodule.c 91.67% <84.61%> (-0.02%) ⬇️
c/tskit/trees.c 94.89% <93.22%> (-0.15%) ⬇️
c/tskit/convert.c 92.04% <100.00%> (ø)
python/tskit/trees.py 97.83% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e622480...02d4771. Read the comment docs.

@jeromekelleher jeromekelleher force-pushed the use-virtual-root branch 2 times, most recently from f5718de to a0498a9 Compare October 16, 2021 09:56
@jeromekelleher jeromekelleher marked this pull request as ready for review October 16, 2021 09:57
@jeromekelleher
Copy link
Member Author

Some useful applications of virtual root here and the new ordering functions. Ready for review, but needs #1704 merged first.

@jeromekelleher
Copy link
Member Author

@Mergifyio rebase

Closes tskit-dev#1787

Note we have left out updating the traversals in vargen for now because
it is undergoing a large refactoring anyway.
Closes tskit-dev#1794

Benchmarks:

Before:
big_tree         0.26075993799986463
many_trees       0.002319710470019345

After:
big_tree         0.004731306999929075
many_trees       2.1276080024108523e-05

So 2-3 orders of magnitude (probably more compared to previous release,
since this using faster node iteration)
@mergify
Copy link
Contributor

mergify bot commented Oct 20, 2021

rebase

✅ Branch has been successfully rebased

@jeromekelleher
Copy link
Member Author

Bumping this @benjeffery - good to get it in to unblock other stuff

Copy link
Member

@benjeffery benjeffery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, LGTM!

@benjeffery benjeffery added the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Oct 20, 2021
@mergify mergify bot merged commit e5dc0f6 into tskit-dev:main Oct 20, 2021
@mergify mergify bot removed the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Oct 20, 2021
@jeromekelleher jeromekelleher deleted the use-virtual-root branch October 20, 2021 11:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants