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

Error when the network has negative or missing branches for phylolm #180

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

pbastide
Copy link
Member

@pbastide pbastide commented Jul 6, 2022

See issue #179.

This just throws an explicit error early to avoid a confusing Cholesky related error later.

Let me know if you see any better way to deal with this, or if I missed something.

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #180 (79ecc71) into master (14ade1d) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
- Coverage   86.03%   85.94%   -0.09%     
==========================================
  Files          29       29              
  Lines       11842    11850       +8     
==========================================
- Hits        10188    10185       -3     
- Misses       1654     1665      +11     
Impacted Files Coverage Δ
src/traits.jl 97.00% <100.00%> (+0.02%) ⬆️
src/moves_semidirected.jl 97.26% <0.00%> (-1.10%) ⬇️
src/phyLiNCoptimization.jl 94.16% <0.00%> (-0.95%) ⬇️

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 14ade1d...79ecc71. Read the comment docs.

@cecileane
Copy link
Member

I'll suggest a few changes. for example, the new function to check for missing / negative branch lengths could be very useful elsewhere.

@cecileane cecileane merged commit 2cf465b into master Jul 8, 2022
@cecileane cecileane deleted the branch_length_error branch August 5, 2022 15:46
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.

2 participants