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

treesim.birth_death_tree() raises division by zero error when used to extend an existing tree #188

Closed
Androstane opened this issue May 15, 2024 · 4 comments · Fixed by #190

Comments

@Androstane
Copy link

Hello,

Thanks for developing the tool! When I was using Treesim to simulate a birth-death tree with multiple stages, an error had occurred. Specifically, when treesim.birth_death_tree() raises division by zero error when the tree argument is supplied. Here's a minimal example:

import random
import dendropy
from dendropy.simulate import treesim

tree = dendropy.Tree()
tree = treesim.birth_death_tree(1.0, 0.5, max_time=random.randint(1, 8), tree=tree, repeat_until_success=True)

When running the code, the following error would arise:

ZeroDivisionError                         Traceback (most recent call last)
<ipython-input-2-a488d6f12df8> in <module>
      3 from dendropy.simulate import treesim
      4 tree = dendropy.Tree()
----> 5 tree = treesim.birth_death_tree(1.0,0.5,max_time=random.randint(1,8),tree=tree,repeat_until_success=True)

[~/anaconda3/lib/python3.8/site-packages/dendropy/model/birthdeath.py](https://vscode-remote+ssh-002dremote-002b7b22686f73744e616d65223a226b696d7572612e63732e726963652e656475222c2275736572223a22796c313833227d.vscode-resource.vscode-cdn.net/home/yl183/sampled_ancestor/preprocessing/~/anaconda3/lib/python3.8/site-packages/dendropy/model/birthdeath.py) in birth_death_tree(birth_rate, death_rate, birth_rate_sd, death_rate_sd, **kwargs)
    329 
    330         # waiting time based on above probability
--> 331         waiting_time = rng.expovariate(rate_of_any_event)
    332 
    333         if ( (gsa_ntax is not None)

[~/anaconda3/lib/python3.8/random.py](https://vscode-remote+ssh-002dremote-002b7b22686f73744e616d65223a226b696d7572612e63732e726963652e656475222c2275736572223a22796c313833227d.vscode-resource.vscode-cdn.net/home/yl183/sampled_ancestor/preprocessing/~/anaconda3/lib/python3.8/random.py) in expovariate(self, lambd)
    493         # we use 1-random() instead of random() to preclude the
    494         # possibility of taking the log of zero.
--> 495         return -_log(1.0 - self.random())[/](https://vscode-remote+ssh-002dremote-002b7b22686f73744e616d65223a226b696d7572612e63732e726963652e656475222c2275736572223a22796c313833227d.vscode-resource.vscode-cdn.net/)lambd
    496 
    497 ## -------------------- von Mises distribution --------------------

ZeroDivisionError: float division by zero
@mmore500
Copy link
Collaborator

Thanks for the report! Do you happen to know if this error occurs when working with a non-empty tree?

As a pointer for myself in returning to this issue, here's a link to this method's documentation
https://jeetsukumaran.github.io/DendroPy/library/birthdeath.html#dendropy.model.birthdeath.birth_death_tree

@Androstane
Copy link
Author

It seems to me that the error would occur when working with empty and non-empty trees. A quick fix I have found is to remove the condition check at:

if getattr(nd, extinct_attr_name, False):

as with it, the list of the extant tips would always be empty even if a non-empty tree is supplied, leading to a division by zero issues later when sampling the event node from the list of extant tips. It is probably because, by default, the is_extinct attribute is not added to the node. I am not sure if this fix would lead to other unexpected performance in the code though.

@mmore500
Copy link
Collaborator

Hi @Androstane --- I believe I found the root cause of the issue and fixed it. The fix will appear in DendroPy 4.6.2 which will release shortly!

Thank you for the report, it is much appreciated 👍

@mmore500
Copy link
Collaborator

Quick update, looks like we need to get the maintainers to add verified email addresses before publishing due to a new PyPI policy. Hope to get the release out soon.

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 a pull request may close this issue.

2 participants