search

sate-dev / sate-core

3 stars 3 forks source link

decomposition problem on unresolved edges #13

Open smirarab opened 12 years ago

smirarab commented 12 years ago

If the tree used to decompose the set of taxa has polytomies with degree larger than maximum subset size, sate divides the poltyomy into two subsets, one with size 1 and the other with side d-1 (where d is the degree of the ploytomy).

If the tree and the polytomy are both very large, this can result in maximum recursion error.

The easiest solution randomly resolving polytomies for any tree inside SATe. This can be achieved easily by adding the following line:

        dendropy_tree.resolve_polytomies(update_splits=True)

as the first line of the constructor of PhylogeneticTree (in tree.py line 24).

This is not a imaginary case. FastTree outputs polotyomies when input sequences are identical. And we have faced this problem on a bunch of real biological datasets we are testing.

If this solution is fine with everyone, I can go ahead and apply it to the code.

Thanks Siavash

mtholder commented 12 years ago

that sounds fine to me.

Mark

On Apr 16, 2012, at 4:17 PM, Siavash Mirarab wrote:

If the tree used to decompose the set of taxa has polytomies with degree larger than maximum subset size, sate divides the poltyomy into two subsets, one with size 1 and the other with side d-1 (where d is the degree of the ploytomy).

If the tree and the polytomy are both very large, this can result in maximum recursion error.

The easiest solution randomly resolving polytomies for any tree inside SATe. This can be achieved easily by adding the following line:

       dendropy_tree.resolve_polytomies(update_splits=True)

as the first line of the constructor of PhylogeneticTree (in tree.py line 24).

This is not a imaginary case. FastTree outputs polotyomies when input sequences are identical. And we have faced this problem on a bunch of real biological datasets we are testing.

If this solution is fine with everyone, I can go ahead and apply it to the code.

Thanks Siavash

Reply to this email directly or view it on GitHub: https://github.com/sate-dev/sate-core/issues/13

mtholder commented 12 years ago

Hi Siavash, I decided that it would be a little odd for a class called "PhylogeneticTree" to modify the tree on initialization. So I added a force_fully_resolved attribute, and I used the resolve_polytomies call in a couple of places (corresponding to when new trees are read).

This should be fixed after commit 73f1d96b2b229dd7b4c12205c7388a97e37ed84f on 26 Apr, 2012. Can you verify that this fix works on a dataset that has this problem (or send the dataset to me or Jamie)?

Thanks

smirarab commented 12 years ago

HI Mark,

Sure, I will test this.

Regards Siavash

On Thu, Apr 26, 2012 at 2:26 PM, Mark Holder < reply@reply.github.com

wrote:

Hi Siavash, I decided that it would be a little odd for a class called "PhylogeneticTree" to modify the tree on initialization. So I added a force_fully_resolved attribute, and I used the resolve_polytomies call in a couple of places (corresponding to when new trees are read).

This should be fixed after commit 73f1d96b2b229dd7b4c12205c7388a97e37ed84f on 26 Apr, 2012. Can you verify that this fix works on a dataset that has this problem (or send the dataset to me or Jamie)?

Thanks

Reply to this email directly or view it on GitHub: https://github.com/sate-dev/sate-core/issues/13#issuecomment-5365661

mtholder commented 12 years ago

Actually hold off for a minute I have a commit that I'm about to push ... On Apr 26, 2012, at 2:42 PM, Siavash Mirarab wrote:

HI Mark,

Sure, I will test this.

Regards Siavash

On Thu, Apr 26, 2012 at 2:26 PM, Mark Holder < reply@reply.github.com

wrote:

Hi Siavash, I decided that it would be a little odd for a class called "PhylogeneticTree" to modify the tree on initialization. So I added a force_fully_resolved attribute, and I used the resolve_polytomies call in a couple of places (corresponding to when new trees are read).

This should be fixed after commit 73f1d96b2b229dd7b4c12205c7388a97e37ed84f on 26 Apr, 2012. Can you verify that this fix works on a dataset that has this problem (or send the dataset to me or Jamie)?

Thanks

Reply to this email directly or view it on GitHub: https://github.com/sate-dev/sate-core/issues/13#issuecomment-5365661

Reply to this email directly or view it on GitHub: https://github.com/sate-dev/sate-core/issues/13#issuecomment-5366006

mtholder commented 12 years ago

oops. bug fix. try with 81bb21984b8aa18df32d108e8c61c9ff73e3bf39 or later (I have to bundle dendropy's resolve_polytomies because the GUI bundle has an old dendropy)