richelbilderbeek / pirouette_article

Article about pirouette, by Bilderbeek, Laudanno and Etienne
GNU General Public License v3.0
0 stars 0 forks source link

Add reference to TESS package #82

Closed richelbilderbeek closed 4 years ago

richelbilderbeek commented 4 years ago
    1. As an initial remark, I imagine the models pirouette support have all been implemented in tested in other packages like geiger, ape, etc.? It might be interesting to list where the tree models are coming from, maybe merging Table 1 with section 6. If such models were implemented from scratch, there must evidence that they're working as intended.

Your assumption is correct: the tree models used in the Bayesian inference are all standard BEAST2 tree models. The phylogenies used in this manuscript are from the DDD [...] and ?TESS [...] R packages.

Giappo commented 4 years ago

@richelbilderbeek I am trying to interpret reviewer's words. What does he mean with "models pirouette supports"? At a first glance I'd think of all the possible tree, clock and site models. As far as tree models are concerned, I think we should mention that they are called by babette, which calls them from BEAST2. So ultimately the responsability of this models working as intended is to attribute to BEAST2. Speaking of clock and site models, when they are used in the inference the same argument holds. When we instead use them to simulate alignments it is a different story. Looking into the package I can go down to "pirouette::create_alignment_params" as the alignment_params contain the sim_tral_fun. For example I can see that "sim_alignment_with_std_nsm" calls "phangorn::simSeq". I suppose that all the "sim_tral_funs" work in the same way, so for clock and site models I think we should refer to phangorn. In general I would set the answer pointing not at the "models" we use, but at the routines we use to simulate alignments and to make the inference, referring respectively to BEAST2 and phangorn. Do you agree?

Giappo commented 4 years ago

Another possible mention could be to TESS in the process of simulating a twin tree. In general the way I would proceed is to mention explicitly the packages we are calling when we explain the pipeline, each at their respective step (phangorn when we explain the alignment part, BEAST2 when we explain the inference, TESS when we explain the twinning).

richelbilderbeek commented 4 years ago

From the reviewers' words ...

It might be interesting to list where the tree models are coming from

I feel he only wants to know where the tree we simulate come from. All trees we simulate are from the DDD package and -indeed!- twin trees from the TESS package.

I agree we use phangorn to simulate an alignment, put I see no evidence that this is his/her point. Same for the babette models.

Whaddaya think?

Giappo commented 4 years ago

Well, I think you are right. I do not understand why he uses this argument only for the tree priors and not for the other models (i.e. clock and site), because it would hold in the same way, imho. But we can just decide to stick to what he refers too and live a happier life.

Giappo commented 4 years ago

I am going to specify that we use the TESS package when simulating the branching times for the twin tree, in the paragraph when we describe it.

richelbilderbeek commented 4 years ago

Cool, thanks!