search

daijiang / phyr

Functions for phylogenetic analyses
https://daijiang.github.io/phyr/
GNU General Public License v3.0
30 stars 10 forks source link

Critical bug in pglmm_compare #88

Closed j-iranzo closed 1 year ago

j-iranzo commented 1 year ago

The code of pglmm_compare has a critical bug in line 289. As a consequence, pglmm_compare produces wrong results if the rows of the original data table were not already sorted as the tips of the tree. An easy way to check that something is wrong is by randomly shuffling the rows of the input data table and running pglmm_compare: different results are obtained each time.

To fix the bug, the inputs of the 'match' function in line 289 should be reversed: data <- data[match(phy$tip.label, sp),]

It is straightforward to check that the original line of code does not sort the rows of data as the tips of the phylogenetic tree, while the corrected line does,

daijiang commented 1 year ago

Thanks!! Fixed it.

arives commented 1 year ago

This was probably my fault in the first place. Thanks for finding it, and sorry for the hassle!

From: Daijiang Li @.> Date: Friday, July 7, 2023 at 11:12 AM To: daijiang/phyr @.> Cc: Subscribed @.***> Subject: Re: [daijiang/phyr] Critical bug in pglmm_compare (Issue #88)

Thanks!! Fixed it.

— Reply to this email directly, view it on GitHubhttps://github.com/daijiang/phyr/issues/88#issuecomment-1625640750, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACYX6LH3NLERAL6EUO3PDTLXPAYOVANCNFSM6AAAAAA2BWMJDI. You are receiving this because you are subscribed to this thread.Message ID: @.***>

rdinnager commented 1 year ago

Maybe we should add a test to make sure this bug doesn't crop up again anywhere? Maybe just a test that randomizes the order of the data and then makes sure that all of the main functions return the same answer?