paternogbc / sensiPhy

R package to perform sensitivity analysis for comparative methods
http://onlinelibrary.wiley.com/doi/10.1111/2041-210X.12990/full
GNU General Public License v2.0
12 stars 4 forks source link

Problem in recombine function #159

Closed caterinap closed 7 years ago

caterinap commented 7 years ago

I was trying to find out why intra_influ_phyglm was giving errors when I found that the problem was coming from recombine. The issue was that when using n.rows<- nrow(x[[1]]) it was assuming that all the elements in the list had the same dimension. But this is not always the case, for instance when a model does not converge and the iteration is skipped. So I changed it in a way that it can rbind datasets of different numbers of rows and it works fine. But then I found a second problem. When the list was like that:

image

Recombine was giving this: image

So, the information about the iteration was not correct and the information on intercept/predictor was lost. So I modified it (commit bee4d76) and the output is now:

image

But it means that we need to revise all summaryof interaction functions because they are probably broken! This might also explain the weird things in your plots @paternogbc (but I am not sure). Very bad: this weekend I am at a wedding (again!) and cannot work on this :(

NB: I also changed listinto sensi.list to avoid to use a function name as argument (commit 46b5224)

paternogbc commented 7 years ago

Hey @caterinap thanks a lot for finding and fixing this problem. I will see and fix the broken summary methods tomorrow. But I guess it won't be too much work. And let's make more tests to be sure recombine is working well on all other cases :)

paternogbc commented 7 years ago

Checking functions for summary / sensi_plot problems. (quick check to see the examples) tree_influ_phylm: OK tree_influ_phyglm: OK intra_influ_phylm: OK intra_influ_phyglm: OK

tree_clade_phylm: OK tree_clade_phyglm: OK intra_clade_phylm: OK intra_clade_phyglm: OK

tree_samp_phylm: OK tree_samp_phyglm: OK but for some reason tree_samp_phyglm is not exported at all! (it's not in NAMESPACE) -- normal? intra_samp_phylm: OK intra_samp_phyglm: OK

tree_intra_phyglm: OK tree_intra_phyglm: OK for these two the summary name was wrong - corrected - now NAMESPACE needs update ...