iqbal-lab-org / gramtools

Genome inference from a population reference graph
MIT License
92 stars 15 forks source link

Segfault in latest unit test of multiple matches #34

Closed iqbal-lab closed 8 years ago

iqbal-lab commented 8 years ago

At commit ef3cf76b59b5e0e3e2de62f9fba18dc4e98f90b2

Add testing of sites object to multiple-matches unit test - segfaults

./unittest_bidir_search_bwd_fwd 
[==========] Running 10 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 10 tests from BackwardSearchTest
[ RUN      ] BackwardSearchTest.NoVariants1
[       OK ] BackwardSearchTest.NoVariants1 (4426 ms)
[ RUN      ] BackwardSearchTest.OneSNP
[       OK ] BackwardSearchTest.OneSNP (11693 ms)
[ RUN      ] BackwardSearchTest.TwoSNPs
[       OK ] BackwardSearchTest.TwoSNPs (338 ms)
[ RUN      ] BackwardSearchTest.Two_matches_one_variable_one_nonvariable_region
[       OK ] BackwardSearchTest.Two_matches_one_variable_one_nonvariable_region (3140 ms)
[ RUN      ] BackwardSearchTest.Two_matches_one_variable_second_allele_one_nonvariable_region
[       OK ] BackwardSearchTest.Two_matches_one_variable_second_allele_one_nonvariable_region (366 ms)
[ RUN      ] BackwardSearchTest.Two_long_sites
[       OK ] BackwardSearchTest.Two_long_sites (1201 ms)
[ RUN      ] BackwardSearchTest.Match_within_long_site_match_outside
[       OK ] BackwardSearchTest.Match_within_long_site_match_outside (331 ms)
[ RUN      ] BackwardSearchTest.Long_site_and_repeated_snp_on_edge_of_site
[       OK ] BackwardSearchTest.Long_site_and_repeated_snp_on_edge_of_site (322 ms)
[ RUN      ] BackwardSearchTest.Multiple_matches_over_multiple_sites
Segmentation fault
This is because the sites object ends up with a dummy entry in it with
zero capacity, ready for the next entry, and my unit test does not expect that.
I'm commiting it as it is for the moment. I need to get my head around whether
the behaviour is correct. I didn't need to allow for such a thing in all the previous unit tests
iqbal-lab commented 8 years ago

Here is the test

` TEST(BackwardSearchTest, Multiple_matches_over_multiple_sites){

//prg=acgacacat5gatag6tagga6gctcg6gctct5gctcgtgataatgactagatagatag7cga8cgc8tga8tgc7taggcaacatctacga                                                                       
test_file2="../test_cases/multiple_matches_multiple_sites.txt";
//read aligns over allele 1 of site 5, the nonvariableregion and allele 3 of site 7                                                                                       
query="tgata";

` After running the backward search, this is what sites looks like

` (gdb) p sites $1 = std::list = {[0] = std::vector of length 0, capacity 0, [1] = std::vector of length 1, capacity 100 = {{first = 7, second = std::vector of length 0, capacity 0}}, [2] = std::vector of length 1, capacity 100 = {{first = 5, second = std::vector of length 1, capacity 1 = {1}}}}

`

So we correctly see 7 first, and fail to spot the overlap with allele 3, because it does not cross the 5 - so it's correct/expected behavious to have this {{first = 7, second = std::vector of length 0, capacity 0}}.

This is also correct, and correctly after the 7 {{first = 5, second = std::vector of length 1, capacity 1 = {1}}

I just wasn't expecting this at the front: [0] = std::vector of length 0, capacity 0,

iqbal-lab commented 8 years ago

Need to read the code - in fact if I spent the time reading the code not bug-raising - er - I might have to go away from the computer and forget what i was doing

iqbal-lab commented 8 years ago

This is idiocy on my part. Fixing.

iqbal-lab commented 8 years ago

Fixed. I forgot there was a match in the nonvariable region, and anyway my test code was wrong. test passes now.

iqbal-lab commented 8 years ago

Wow, github clever enough to auto close this bug on the basis of the comment on that commit!!!