KlausVigo / phangorn

Phylogenetic analysis in R
http://klausvigo.github.io/phangorn/
204 stars 38 forks source link

RF distance inaccuracy #140

Closed kbhoehn closed 2 years ago

kbhoehn commented 2 years ago

Hello,

Thanks for the great package. I've been using the RF.dist function and found that it gives a high distance for identical tree topologies. I've attached a tree that reproduces this behavior here: bad_tree.zip

Using these commands, the same tree after ladderization has an RF distance of 20 compared to the original. I would expect that to be 0. Plotting the trees side-by-side shows no meaningful difference in topology so I don't think it's an issue with ladderizing. Do you know what could causing this? It occurs in both the released phangorn and the development version.

Best, -Ken

devtools::install_github("KlausVigo/phangorn")
library(phangorn)
library(ape)

tree = readRDS("bad_tree.rds")
reorder = ladderize(tree)
RF.dist(tree, reorder)

par(mfrow=c(1,2))
plot(tree)
plot(reorder)
KlausVigo commented 2 years ago

Dear @kbhoehn, the issue is that "tree" has an argument order

> attr(tree, "order")
[1] "postorder"

However the tree is not in the format the function expects it, i.e. the format ape:::reorder(tree, "postorder") . Problem is that ape:::reorder(tree, "postorder") just returns the tree if the order argument agrees to avoid unnecessary computations.

> attr(tree, "order") <- NULL
> RF.dist(tree, reorder)
[1] 0

How did you get the tree and which transformations did you, maybe unintended to it. Some functions might reorder the edge matrix, but do not update the order argument.
Kind regards, Klaus

kbhoehn commented 2 years ago

That fixes it. I didn't realize there was an order attribute. I had built the tree with phangorn::pratchet, but I have a custom re-rooting function that doesn't reset this attribute. I'll update it. Thanks!

Best, -Ken