ekg/freebayes — pull request #110 by tsibley (2014-10-24 01:45:17+00:00)
Title: Correct a bunch of off-by-one errors related to targets/positions

*See the test script I used to demonstrate the issues in the first commit. Below is the commit message of the primary and last commit (though there are three commits, at least two of which should be merged/cherry-picked).*

A number of previous commits which switched the internally-used position numbering were not comprehensive. This lead to a clear inconsistency in position handling that resulted in mishandled edge cases.

I audited the code for all uses of left and right boundaries, as well as currentPosition. While it's quite possible I still missed a couple spots, this commit fixes a number of places missed in previous commits. Most of the code, and the intent described in the latest commit messages, uses 0-based, fully-closed positions internally (e.g. 0-1 describes the first two bases). This is different from the external expections of BED and chr:X-Y positions, which both use 0-based, half-open positions (e.g. 0-1 describes just the first base).

I attempted to normalize all such internal arithmetic and comparison to expect 0-based, fully-closed positions, while still expecting the standard 0-based, half-open positions from user input. The edges of freebayes convert to the internal representation as soon as possible to avoid leaking the external representation.

Debugging and error messages printed to the user regarding targets/regions are normalized to be in the 0-based, half-open style so they match the input the user may have given. Debugging and error output regarding currentPosition still mixes reporting 0-based numbers with 1-based numbers. It appears to fall more or less along the lines of "am I reporting about a target (0-based)?" vs. "am I reporting about a VCF position (1-based)?", but the utility of the trace logs and other debugging output is unclear to me, so I left it alone for now. I audited the two cases using the following greps:

git grep -P '<<[^<]*?currentPosition(?!\s*\+\s*1)'
git grep -P '<<[^<]*?currentPosition(?=\s*\+\s*1)'

It's worth noting the hunk in this commit which removes a check on parameters.allowIndels, because this parameter previously affected internal position tracking in interesting ways: when enabled, positions were incremented by 0.5 (half a base) during each cycle. This code was removed in 6835a00, however, which is good since it added notable complexity to position comparisons.