Ready for the first release?

[thibautjombart]

@KlausVigo @zkamvar @emmanuelparadis looks like we're getting ready to put this boy on CRAN. Here's a check list of things potentially remaining to do - I don't create issues yet as some of them we may just discard or keep for later.

• [x] finish vignette: mainly adding @KlausVigo 's stuff and tidying up
• [x] add to vignette: demo of getTree
• [x] make sure current phangorn is released before
• [x] pass check on current devel version
• [x] add a function to get trees for all DNA matrices (separate issue)

[KlausVigo]

@thibautjombart I submitted phangorn to CRAN and it made it there in no time. I started writing the vignette, I will do a pull request tomorrow. I can't get the vignette compile on my machine, so you need to check if it runs.

[thibautjombart]

@KlausVigo that's awesome for phangorn; as for the vignette, it should run on any platform, as we don't even need latex; what is the error message?

[KlausVigo]

@thibautjombart I just could not knit the .Rmd file from within RStudio. Was working fine if I installed it. The data set we use, has the problem that the NJ trees have lots of negative edge weights, which plot really badly. Should we set negative edge weights to zero in getTree?

[zkamvar]

We ran into this issue with poppr. Here is an implementation from Kuhner and Felsenstein (1994)

[thibautjombart]

@KlausVigo Yeah I think because it is a vignette, and not a typical .Rmd, we need to compile it while building the package. Or use devtools:

> library(devtools)
> build_vignettes()
Building apex vignettes
Moving apex.html, apex.R to inst/doc/
Copying apex.Rmd to inst/doc/

Does this work for you?

[thibautjombart]

As for the branch length, yeah let's put them to 0.. or use a different one? Any idea?

[zkamvar]

I didn't have time to elaborate before: Basically in Kuhner and Felsenstein (1994), they say that if you have an nj tree, it must be corrected by setting the negative branch to zero and then reducing the sibling branch by that amount (Page 465, second to last sentence in Correction of Negative Branch Lengths).

I have already written this and can add this in and have a conditional for nj trees if you would like.

[thibautjombart]

Looks cool. But I think this belongs to ape. And usually when I say that, I end up realising Emmanuel implemented that xxx years ago ;) @emmanuelparadis thoughts?

[thibautjombart]

@KlausVigo I still currently get an error in the vignette (line 201):

> pp <- pmlPart(bf ~ edge + nni, z, rooted=TRUE)
[1] -3602.931
loglik: -4433.69 --> -3602.931 
Error in optim.pml(fits[[i]], optNni = PartNni, optBf = PartBf, optQ = PartQ,  : 
  Tree must be ultrametric!
> 

[thibautjombart]

There was a call above that where 'z' was not defined. I have defined it as multiphyDat but unsure if that's what you need. From reading the doc it looks like it should be a pml?

[emmanuelparadis]

Mon, 13 Apr 2015 03:34:46 -0700 Thibaut Jombart notifications@github.com:

Looks cool. But I think this belongs to ape. And usually when I say that, I end up realising Emmanuel implemented that xxx years ago ;) @emmanuelparadis thoughts?

It's not in ape. Others (e.g., Kumar) simply recommend to set the negative branch lengths of NJ trees to zero. So something like this would do the trick:

tr <- nj(.... tr$edge.length[tr$edge.length < 0] <- 0

[thibautjombart]

I'm happy with that!

[KlausVigo]

@thibautjombart are you at the moment working on apex.Rmd or can I push some changes / bug fixes?

[thibautjombart]

lemme check.. nope, I pushed my last stuff, you can go for it :)

On Mon, Apr 13, 2015 at 5:33 PM, klash notifications@github.com wrote:

@thibautjombart https://github.com/thibautjombart are you at the moment working on apex.Rmd or can I push some changes / bug fixes?

— Reply to this email directly or view it on GitHub https://github.com/thibautjombart/apex/issues/8#issuecomment-92419400.

[KlausVigo]

done, should look nicer now.

[thibautjombart]

Cool! Looks good to me, except for the negative branch length. I'm tempted to add an argument to getTree like fixBranchLength=TRUE by default..

[thibautjombart]

Added the feature as of a5537de1b663420ad38d834073f0a250b1713c3f