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

Write pir_run_distribution #389

Closed Giappo closed 4 years ago

Giappo commented 4 years ago

I will enjoy it in the pirouette examples!

richelbilderbeek commented 4 years ago

Giappo will love it, if I do this ASAP :+1:

richelbilderbeek commented 4 years ago

I've written the tests, @Giappo will be happy to take over :+1:

Giappo commented 4 years ago

Hi @richelbilderbeek ,

I saw that in the test you expect:

1) the input to be a list of pir_params (aka pir_paramses); 2) the output to be a list of pir_outs;

What if instead we make it even easier? Something like:

1) input becomes only one pir_params and we require an additional "n_replicates" argument. Then we automatically generate the pir_paramses replicating the original pir_params n times, but changing all the rng seeds (e.g. in twinning) when needed. 2) the output becomes just one pir_out, as if we had only one big run. The advantage is that this pir_out can be immediately plugged into pir_plot to produce an output

Lemme know what you think

richelbilderbeek commented 4 years ago

Hi @Giappo,

Thanks for your thoughts on this!

  1. input becomes only one pir_params and we require an additional "n_replicates" argument. Then we automatically generate the pir_paramses replicating the original pir_params n times, but changing all the rng seeds (e.g. in twinning) when needed.

I see your point, but have already thought about this. I would agree if the user would never care about the intermediate files created. We, as users, do. Don't forget: each pir_params contains all files' paths.

But I think the idea is good, so instead, I would favor an extra function that does what you suggested to do for the user, called something like create_pir_paramses or replicate_pir_params. Go ahead and write that one :+1:

the output becomes just one pir_out, as if we had only one big run. The advantage is that this pir_out can be immediately plugged into pir_plot to produce an output

You, as the pir_out/pir_outs master, I will follow your lead. Go ahead and so do :+1:

richelbilderbeek commented 4 years ago

Cool, will check :+1:

richelbilderbeek commented 4 years ago

Check, well done :+1: