Added the ability for Indels to be recovered from dangling heads

[jamesemery]

In helping @bhanugandham figure out why a particular site was failing it became apparent that merging dangling head code was failing to recover deletions in the dangling head. Furthermore there is some code in the dangling end recovery code that asserts a certain high standard of matching (usually 1 but sometimes dangling branch length/kmersize) getMaxMismatches(final int lengthOfDanglingBranch). Both of these facts seem likely to cause dangling heads to be dropped despite their being still potentially informative, particularly the indel code.

I have added the ability for the index recovery code to account for the cigar string when merging dangling ends. Addtionally rather than counting mismatches to reject the branch it simply requires a minimum matching end (which can be changed, I suspect this is where the lionshare of the differences come from). Unfortunately changing the tests is non-trivial (as this happened to change the integration test results for HaplotypeCaller at a few sites) so I wanted to get this branch up to solicit advice a to whether it is worth pursuing this fix. @davidbenjamin @ldgauthier @droazen

[davidbenjamin]

Mostly little stuff, with one comment that might simplify the prefix code.

[davidbenjamin]

delete?

[davidbenjamin]

Instead of tracking matchesSinceLastMismatch as you go, why not just find the two offsets and then backtrack from there in both sequences, looking for the required number of exact matches?

[jamesemery]

@davidbenjamin I have refactored this branch to account for changes to the codebase adjacent to this code. In the interest of not possibly harming any of the old results I have made this a toggle and I have also made the setting apply symmetrically to tails and heads and added a few simple tests in the existing framework.

[gatk-bot]

Travis reported job failures from build 30561
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	openjdk11	30561.12	logs
unit	openjdk8	30561.3	logs

[davidbenjamin]

I propose a simplification to the code. Do you think it works?

[davidbenjamin]

typo

[davidbenjamin]

inline this?

[davidbenjamin]

It looks like this can be made much simpler by traversing from the end of the paths. I think the pseudocode in this case becomes:

• go backwards from the right until you hit a mismatch or indel.
• if this covered fewer than getMinMatchingBases() return (-1,-1).
• otherwise return the read and ref position you got to.

In code this looks something like

refIdx = path1.length - 1;
readIdx = path2.length - 1;
for (final CigarElement ce : Lists.reverse(cigarElements)) {
   if (!(ce.consumesReadBases() && ce.consumesRefBases()) {
      break;
   }
   for (int j = 0; j < elementLength; j++, refIdx—; readIndx—) {
      if (path1[refIdx] != path2[readIdx]) {
         break;
      }
   }  
}

final int matches = path2.length - readIdx;
return matches < minimum ? (-1,-1) : (readIdx, refIdx);

[jamesemery]

Right, your previous comment got lost in the rebase. I just checked and there is a problem with this, namely that there is no guarantee that the ref path and read path match in length. Thus in order to match the bases at the end of the dangling path I have at the very least parse through the cigar once forwards and then start walking it backwards... Perhaps that is still cleaner than what I wrote though...

[jamesemery]

It looks like we excise the trailing deletions that might be useable for quickly calculating the end base this at on this line: return new DanglingChainMergeHelper(altPath, refPath, altBases, refBases, AlignmentUtils.removeTrailingDeletions(alignment.getCigar()));

[davidbenjamin]

Back to @jamesemery

[gatk-bot]

Travis reported job failures from build 30869
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	openjdk11	30869.13	logs
integration	openjdk11	30869.12	logs
variantcalling	openjdk8	30869.4	logs
unit	openjdk8	30869.3	logs
integration	openjdk8	30869.2	logs

[gatk-bot]

Travis reported job failures from build 31570
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	openjdk11	31570.13	logs
variantcalling	openjdk8	31570.4	logs
unit	openjdk8	31570.3	logs

[gatk-bot]

Travis reported job failures from build 32140
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
cloud	openjdk8	32140.1	logs
unit	openjdk8	32140.3	logs
variantcalling	openjdk8	32140.4	logs

[ahaessly]

fix spelling. change to minMatchingBasesToDanglingEndRecovery

[jamesemery]

@davidbenjamin Do you think you can take another look at this? It would be nice to finally merge this code into the next release next week.

[gatk-bot]

Travis reported job failures from build 32609
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	openjdk11	32609.13	logs
unit	openjdk8	32609.3	logs
variantcalling	openjdk8	32609.4	logs

[gatk-bot]

Travis reported job failures from build 32612
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	openjdk11	32612.13	logs
unit	openjdk8	32612.3	logs

[davidbenjamin]

@jamesemery Looks good.