richelbilderbeek / pirouette

R package that estimates the error BEAST2 makes from a given phylogeny
GNU General Public License v3.0
3 stars 2 forks source link

Cannot create branching times for a Yule tree #342

Closed richelbilderbeek closed 4 years ago

richelbilderbeek commented 5 years ago

pirouette provides for the creation of Yule twin trees, thus it should be possible to create a set of branching times for a Yule tree. A Yule tree has, by definition, an extinction rate of zero.

Whenever I use that knowledge to call create_twin_branching_times I get an error:

Error in tess.sim.taxa.age.constant(n, nTaxa, age, lambda, mu, massExtinctionTimes,  : 
  Invalid parameter values for lambda and mu!

Here is the test to reproduce the error with:

test_that("use", {

  skip("Cannot create branching times for a Yule tree, #342")
  phylogeny <- ape::read.tree(text = "(((A:1, B:1):1, C:2):1, D:3);")

  # Error in this statement
  yule_branching_times <- create_twin_branching_times(
    seed = 314,
    lambda = 0.0,
    mu = 1.0,
    phylogeny = phylogeny,
    n_replicates = 1,
    method = "random_tree"
  )
  bd_branching_times <- create_twin_branching_times(
    seed = 314,
    lambda = 0.1,
    mu = 1.0,
    phylogeny = phylogeny,
    n_replicates = 1,
    method = "random_tree"
  )
  expect_equal(yule_branching_times, bd_branching_times)
})

There are many ways to solve this.

Good luck :+1:

richelbilderbeek commented 4 years ago

As the man that eats branching times for breakfast, I assign @Giappo to this one :+1:

Giappo commented 4 years ago

@richelbilderbeek I do not understand this issue. For two reasons:

1) If you want a yule process you need to set the parameters in such a way that lambda > 0 and mu = 0. In the example you proposed lambda = 0 and mu > 0. I do not see how this can be working.

2) Why would you expect a yule process and a bd process to have the same branching times? Different processes are expected to return different branching times.

My take on this is: this test is not correct. If you want me to do something else write a test and I will work on it.

richelbilderbeek commented 4 years ago

If you want a yule process you need to set the parameters in such a way that lambda > 0 and mu = 0. In the example you proposed lambda = 0 and mu > 0. I do not see how this can be working.

You are indeed right: I switched lambda and mu! I will create another Issue to rename these to speciation_rate and extinction_rate.

Closing this Issue, sorry for the noise :+1: