Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent results from qsort callback #11

Closed
yugr opened this issue May 19, 2018 · 2 comments
Closed

Inconsistent results from qsort callback #11

yugr opened this issue May 19, 2018 · 2 comments

Comments

@yugr
Copy link

yugr commented May 19, 2018

Hi,

qsort callback hit_sorter_by_evalue may return random result when input structs have all compared fields equal. This in turn may causes inconsistent order or even crashes in some qsort implementations.

The issue has been detected when running standard testsuite under SortChecker:

cmsearch[23540]: qsort: comparison function is not symmetric (comparison function 0x4ab689 (/home/yugr/build/infernal-1.1.1/src/cmsearch+0x4ab689), called from 0x4abb66 (/home/yugr/build/infernal-1.1.1/src/cmsearch+0x4abb66), cmdline is "../src/cmsearch -E 0.1 --tblout esltmpaa22822.OUTFILES.tbl esltmpaa22822.OUTFILES.cm esltmpaa22822.OUTFILES.fa")

Closer examination in debugger reveals that hit_sorter_by_evalue returns returns the same value for equal results:

(gdb) p hit_sorter_by_evalue(&h->hit[2], &h->hit[3])
$15 = -1
(gdb) p hit_sorter_by_evalue(&h->hit[3], &h->hit[2])
$16 = -1

The fix is simply to replace final comparison

return  (h1->pass_idx < h2->pass_idx ? 1 : -1 ); /* fourth key, pass_idx, high to low */

with

return  (h1->pass_idx < h2->pass_idx ? 1 : h1->pass_idx > h2->pass_idx ? -1 : 0); /* fourth key, pass_idx, high to low */

A similar issue seems to exist in most other hit_sorter* callbacks .

@cryptogenomicon
Copy link
Member

Thanks, I agree. We'll have a look.

@nawrockie
Copy link
Member

nawrockie commented May 23, 2018

Thanks, yugr. This has been fixed in the develop branch as of commit 2c83262. The fix will be included as part of the next release (which will be the next commit on master), but it is currently unclear when that will be.

For changes responsible for the fix, see:
5e095af...2c83262

Ignore the addition of the bug-i47.pl script -- that is unrelated.

cryptogenomicon added a commit to EddyRivasLab/hmmer that referenced this issue Nov 7, 2018
A qsort() helper function needs to return 0 when its keys are
equal. We had multiple places where qsort helpers are only returning
-1/1. First reported in EddyRivasLab/infernal#11 by Yuri Gribov
(yugr), author of SortCheck. In May 2018, EPN committed Infernal
fixes, and sent a pull request (#139) with H3 fixes. Belatedly, here
are the analogous H4 fixes.

Also, adds -L flag to the find's in makeTAGS.sh, so the TAGS file
includes lib/easel when that's a symlink.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants