search

richelbilderbeek / pirouette_article

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

Implement feedback Rampal #1 #42

Closed richelbilderbeek closed 5 years ago

richelbilderbeek commented 5 years ago

From @rsetienne:

I have started to edit the overleaf document. I like the general setup with ERQs, But I got lost in the tables. I find the naming of the parameters/models not very intuitive. For example, I get confused between inference and generative models, and some parameters have a totally different type of meaning for a different subscript, for example m_r means mutation rate, but m_s means MCMC settings. I would recommend G_c for generative clock model and similarly G_s for generative site model and G_tp for generative tree prior model. Then you can use the same subscripts for the inference model, i.e. I_c, I_s, I_tp. And I can go on for almost all symbols used. Please rethink this.

Skimming a little further, I did not understand what you meant by "If the attached inference model $I$ is the generative model, then it should always be selected. If, instead, the attached inference model is a candidate model, the user can either let \verb;pirouette; select it as the best among all the candidate models, or force its selection (regardless of it being the best)." I did not read further. Please revise these sections with all this in mind, and then I will look at the revision again.

Giappo commented 5 years ago

So, I updated the figure to include Rampal's suggestions and make it more explicit to the reader (I hope you will like it). Then I updated the table to be, on one hand, in agreement with the figure and, on the other hand, to be more connected to the code we provide in listings 2, 3 and 5. Additional arguments are not shown. If you want more arguments to be presented in the table, just add them to the listings and we can add them. Now I am updating the whole main text to be in agreement with the table. To be honest, I feel quite satisfied by the way it's looking.

Giappo commented 5 years ago

I have two doubts: 1) Shall we call the variables in the main text as we call them in the table (e.g. site_model, tree_prior) to create a clearer link with tables and listings? 2) The twinning process always calls the inference model tree prior, so do we actually need to specify it? In the figure I removed already. Maybe it's a good idea to make also the main text simpler... @richelbilderbeek lemme know what you think.