KlausVigo / phangorn

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

`ancestral.pars` fails if called for `ACCTRAN` due to internal code within `ptree` expecting a matrix and getting a list of vectors #112

Closed dwbapst closed 3 years ago

dwbapst commented 3 years ago

Hi Klaus,

Here's a reproducible example with the latest release of phangorn:

library(ape)
set.seed(444)
tree <- ape::rtree(50)
#simulate under a likelihood model
char <- ape::rTraitDisc(tree, 
            k = 3, rate = 0.7)
tree$edge.length <- NULL
tree <- ape::ladderize(tree)
anc <- phangorn::ancestral.pars(tree, char1, 
            type = "ACCTRAN", cost = cost)

Here's the resulting console output and some diagnosis:

> anc <- phangorn::ancestral.pars(tree, char1, 
+             type = "ACCTRAN", cost = cost)
Error in rowSums(X) : 'x' must be an array of at least two dimensions
> packageVersion("phangorn")
[1] ‘2.6.2’
> traceback()
6: stop("'x' must be an array of at least two dimensions")
5: rowSums(X)
4: FUN(X[[i]], ...)
3: lapply(res, fun)
2: ptree(tree, data, return = return)
1: phangorn::ancestral.pars(tree, char1, type = "ACCTRAN", cost = cost)

The issue traces to internal function ptree, where the issue can be solved rather easily. I will put those changes on a forked branch and make a pull request to the master. I also discovered what seems to be weirdly repeated code in ptree.

dwbapst commented 3 years ago

I raise this issue as it is causing some problems for some dont-run examples for paleotree but that's an issue of minor consequence at the moment.

KlausVigo commented 3 years ago

Hi @dwbapst, can you please check if c7581d5 fixed your problem on your setup?

dwbapst commented 3 years ago

Yes that fixes the issue, thanks!