Repository navigation
indel search: walk each side of a seed once, not once per budget split - #36
Merged
Merged
Conversation
… split extend_bidirectional ran dfs for every split of the indel budget between the two sides of a seed: 2 passes x (n+1) splits x 2 directions, so 12 walks per candidate position with 2 indels and 8 with 1, and every node of every walk allocated a Vec. dfs now walks once at the full budget and records (protein chars consumed, edits used) per path into a reused list; each split takes the entries with at most its share of edits. The budget only decides whether an edit branch is open, so those entries are exactly what a walk at the smaller budget returns, in the same order, and the pairing loops emit the same windows in the same order. Windows are (start, end) positions instead of copied sequences, deduped against a short list, and indel_search_peptide dedups on (protein, start, end) and builds the matched string only for hits it keeps. Output is byte-identical to before. The old and new extension code agree side by side on every case over a 2- and 3-letter alphabet up to query 7 and protein 10 (608M calls) and on 30M random repeat-rich cases; rs_indel_match output is identical on 123 jobs over the human proteome (every cached k, 0 to 3 indels, the benchmark files, repeat tracts, protein ends, at 1 thread and all); the 29-scenario human comparison and all 11 indel benchmark sets match master with 100% recall. 2-indel synthetic search: 13.3 s -> 7.1 s.
…list each path end once Two exact shortcuts on top of the single walk. Most candidate seed positions fail on the right side, and with no right path there is nothing to pair, so the left walk is skipped. And within one pass the protein consumed follows from the edit count (one fewer per deletion, one more per insertion), so a later path ending in the same (consumed, edits) could only re-emit windows its first copy already emitted; each pair is now listed once, which keeps repeat-rich proteins from multiplying the pairing loops. Output is byte-identical: the same side-by-side check against the old extension code (608M exhaustive and 30M random cases), identical rs_indel_match output on all 123 human-proteome jobs at 1 thread and all, the 29-scenario comparison and all 11 indel benchmark sets identical to master with 100% recall.
The brute-force property tests compare sets of matches, so they cannot see a change that reorders, duplicates or relabels rows without changing the set. This records indel search output row for row (every column, row order) for 53 queries against twelve proteins built around repeat tracts, two of them isoform-like so hits on both share coordinates: exact windows, one and two edits of each kind, windows inside homopolymer and short repeats and at protein ends, a motif that occurs twice, a mixed edit that is never found, short peptides and decoys. Four runs: 1 and 2 indels, with the derived k and with k=2. Recorded from the engine before the single-walk change. An engine that pairs the budget splits in another order (same rows, new order) or dedups hits without the protein passes the whole existing suite; both fail this test, and the last six queries were chosen for that. The file has its CI matrix row.
Collaborator
|
Merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two-indel search spent almost all its time extending seeds. A 3-mer seed shows up about 3,700 times in human, and at every one of those spots extend_bidirectional ran dfs once for every way of splitting the indel budget between the two sides of the seed, with a new Vec allocated at every dfs node, even at dead ends:
Now each side is walked once per pass at the full budget, and every path records how many edits it used. A budget split just takes the paths that fit. The budget only decides whether an edit branch is open, so those are exactly the paths a smaller budget walk returns, in the same order, and the windows come out in the same order too. On top of that the left walk is skipped when the right one finds nothing, each (consumed, edits) path end is kept once (within a pass the protein consumed follows from the edit count), and windows are (start, end) positions instead of copied sequences.
Engine instructions (callgrind) drop 2.4x for 1 indel and 3.3 to 4.2x for 2 indels, including 2-mer seeds and repeat tracts. In wall time a 2-indel search of 20k synthetic queries goes from 30.0 s to 13.9 s, and the 1-indel engine on 50k queries from 3.3 s to 2.3 s.
Output is byte-identical, not just the same matches:
New test_indel_golden.py (in the CI matrix) checks indel output row for row, every column and the row order. The property tests compare sets of matches, so an engine that reorders rows or merges hits on two proteins at the same coordinates passes all of them; both fail this one.
Also gets rid of clippy's too_many_arguments warning on dfs.