Preserve complete and ordered DMR segmentation - #703
Closed
SuhasSrinivasan wants to merge 4 commits into
Closed
Conversation
Contributor
Author
|
This completed segmentation fix is now consolidated into #704 with the related DMR denominator, replicate, and positive-coverage corrections. The exact combined head received independent Rust and Bioinformatics review and passed the full serial workspace gate (208 passed, 14 ignored, 0 failed). Closing this duplicate review surface so DMR science can be reviewed in one PR. |
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
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.
Fixes #686.
Fixes #695.
Summary
Severity
Severity: High — scientific correctness, output completeness, and reproducibility
Rationale: The decoder silently omitted one scored site per HMM chunk, and unordered contig collection could close and reopen a contig at internal batch boundaries. Both alter segment boundaries, counts, aggregates, effects, and state continuity on accepted input.
Root cause
The Viterbi backtrace replaced the final emitted state with its predecessor and encoded an N-1 path/position assumption. Separately, process_batch_of_positions collected outer FxHashMap entries directly, replacing the scheduler's lexical order with hash iteration before the stateful receiver.
Implementation
Preserved behavior
Non-goals
Behavior before and after
Testing
Environment: macOS 26.6 arm64; rustc/cargo 1.90.0; installed modkit 0.6.4 reference; samtools/bgzip/tabix 1.23.1; ignored test-only Cargo.lock SHA-256 78876ab4a98da30caad167744d1c8a0875c27edad7b0b3ad5f8a1891f78604ea resolving hts-sys 2.2.0. Cargo.lock is not in the diff.
Tests not performed
Scientific validation
Output and compatibility
Reviewer guide
Checklist