Update match function

[mr-eyes]

Pushing back search2 result to vector v if it's kmer exist and found2 is already true.

[mschilli87]

This seems to overlap with #166.
@abuelanin: Can you explain what is wrong with the current code and how your changes fix it?

Current master seems to contain a definition of val3 at

kallisto/src/KmerIndex.cpp

Line 1124 in 554da09

KmerEntry val3;

that doesn't appear to be used anywhere.

Now you re-indroduce it with

kallisto/src/KmerIndex.cpp

Lines 1103 to 1104 in bfa8edd

	KmerEntry val3 = search2->second;
	v.push_back({valE, kit2->second});

and replace it whit valE in

kallisto/src/KmerIndex.cpp

Lines 1103 to 1104 in 256efcf

	KmerEntry valE = search2->second;
	v.push_back({valE, kit2->second});

while still leaving the unused val3 in

kallisto/src/KmerIndex.cpp

Line 1127 in 256efcf

KmerEntry val3;

around. 😕

[mr-eyes]

@mschilli87

Short explanation

In the following lines at kmerindex::match() function the right logic in the if condition is followed.
Lines: L1069, L1128, L1178, L1195
!= kmap.end() is equivalent to // if k-mer found
and if != kmap.end() returns true that means search variables contains result of successfull alignment in the hashTable so the following Line#1100 the found2 flag state changed to be true as an indication that search2 has kmer match while the condition == kmap.end() check if kmer not found then raise the flag.
So, in the Line#1099 the condition needs to be fixed to check if the search2 has kmer match by changing the == to !=

Summary: Check if kmer is found instead of checking if the kmer is not found

---

Detailed explanation

Data used in the test

1- Reference

File name	Description	Visualize TDBG
Reference 1	Four Unique Transcripts	GFA
Reference 2	Reference 1 + Deletion from Tr1	GFA
Reference 3	Reference 2 + Fusion between Tr2 & Tr3	GFA

2- Reads

File name	Description	Reference Index
1.fa	R1 ⊂ Tr1, R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4	reference1.idx
2.fa	R1 ⊂ Tr1, R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4, R5 ⊂ Tr5	reference2.idx
3.fa	R1 ⊂ (Tr1 & Tr5), R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4, R5 ⊂ Tr5	reference2.idx
4.fa	R1 ⊂ Tr1, R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4, R5 ⊂ Tr5, R6 ⊂ Tr6	reference3.idx
5.fa	R1 ⊂ Tr1, R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4, R5 ⊂ Tr5, R6 ⊂ (Tr6 & Tr2)	reference3.idx
6.fa	R1 ⊂ Tr1, R2 ⊂ Tr2, R3 ⊂ Tr3, R4 ⊂ Tr4, R5 ⊂ Tr5, R6 ⊂ (Tr2 & Tr3)	reference2.idx

Each read is 100pb

The unexpected behavior happends when pseudo-aligning the 6.fa with reference1 which has the original 4 unique transcripts.

3- Results

Vector V after pseudo-alignment

Original master

(Debugging_Print) SORTED KmerEntry vector(V)

KmerEntry: pos:91 | contig: 2 | contig_Length: 2588 | read_pos: 0
KmerEntry: pos:92 | contig: 2 | contig_Length: 2588 | read_pos: 1
KmerEntry: pos:93 | contig: 2 | contig_Length: 2588 | read_pos: 2
KmerEntry: pos:94 | contig: 2 | contig_Length: 2588 | read_pos: 3
KmerEntry: pos:95 | contig: 2 | contig_Length: 2588 | read_pos: 4
KmerEntry: pos:106 | contig: 3 | contig_Length: 965 | read_pos: 2496

The last read_pos is equal to 2496 while the read length is 100pb !

After the edit !=kmap.end()

KmerEntry: pos:91 | contig: 2 | contig_Length: 2588 | read_pos: 0
KmerEntry: pos:91 | contig: 2 | contig_Length: 2588 | read_pos: 0

After the edit !=kmap.end() & v.push_back({search2->second, kit2->second});

KmerEntry: pos:91 | contig: 2 | contig_Length: 2588 | read_pos: 0
KmerEntry: pos:91 | contig: 2 | contig_Length: 2588 | read_pos: 0
KmerEntry: pos:139 | contig: 3 | contig_Length: 965 | read_pos: 69

That makes sense, there's no need in this case to enter the this is weird, let's try the middle k-mer section or to backOff

---

Side Note

This bug was discovered unintentionally when adding the option Union of Compatibility classes instead of the default Intersect

To be able to create new compatibility classes if there's no intersection between two transcripts in the reference just like when pseudo-aligning the 6.fa on reference1 or reference2

[mschilli87]

@abuelanin: Thx for the detail. However, there are still two PR (this one and #166) suggesting similar fixes for the same problem. Which one do you want to see merged in the end?

[mr-eyes]

@mschilli87 I created #166 and it's updated in my forked repo, also It's more clean and straightforward, this PR has problems as the merge has unknown repository now.
So I suggest closing this PR and merge #166

Thanks.