matsen / pplacer

Phylogenetic placement and downstream analysis
http://matsen.fredhutch.org/pplacer/
GNU General Public License v3.0
74 stars 18 forks source link

cut out partial solutions whose mv_dist is smaller than the closest leaf in the proximal part of tree #232

Closed matsen closed 12 years ago

matsen commented 12 years ago

Here is some low-hanging optimization fruit for voronoi.

Simply build a map first from each node x to the distance to the closest leaf in the part of the tree that is proximal to x. This map is undefined for the root.

Using that map, it's easy and quick to build a function that will give the distance to the closest proximal leaf for any position on the tree.

From there, do not allow partial solutions to be added that have mv_dist smaller than that closest distance.

habnabit commented 12 years ago

I just checked; we're already doing this. https://github.com/matsen/pplacer/blob/04a9ae527cfe9ffbd626ef7ec276370951fe87aa/pplacer_src/voronoi.ml#L654

Unless you want to also do this when combining?

matsen commented 12 years ago

Well, it's hard for me to tell given the lack of comments.

In mark_map:

Also, please put in a comment for that inequality you point to on L654, because if closest_leaf is in the proximal direction then it seems to me that the inequality is going the wrong way.

Yes, we could do the same while combining, but it seems like it will be best just to do it as part of a general culling step.

On Fri, Feb 24, 2012 at 6:30 PM, Aaron Gallagher reply@reply.github.com wrote:

I just checked; we're already doing this. https://github.com/matsen/pplacer/blob/dev/pplacer_src/voronoi.ml#L654

Unless you want to also do this when combining?


Reply to this email directly or view it on GitHub: https://github.com/matsen/pplacer/issues/232#issuecomment-4169660

Frederick "Erick" Matsen, Assistant Member Fred Hutchinson Cancer Research Center http://matsen.fhcrc.org/

habnabit commented 12 years ago

mv_dist is obsolete. See #231.