Harden UMI correctness, SAM/BAM I/O, and performance - #37
Open
justinblethrow-cloud wants to merge 7 commits into
Open
Harden UMI correctness, SAM/BAM I/O, and performance#37justinblethrow-cloud wants to merge 7 commits into
justinblethrow-cloud wants to merge 7 commits into
Conversation
Opening a BAM file is an expensive operation as the index needs to be fully read. In paired reads mode, at every contig change, the file was being opened again to iterate over all reads from the previous contig. This is usually not an issue for genome alignments, but transcriptome alignments may have ~100k contigs, which makes this an expensive operation. Ideally, the two-pass mode should not have to read the file again, and instead just maintain a rolling window of reads in memory.
justinblethrow-cloud
force-pushed
the
upstream/consolidated-dumi
branch
from
July 15, 2026 03:40
ad613d2 to
b4f0a2a
Compare
Optimize UMI parsing, directional clustering, and NgramBKTree internals without adding runtime dependencies. Add an opt-in bounded-window path for coordinate-sorted single-end SAM/BAM input with guarded fallback and atomic output promotion. Harden launchers, dependency verification, artifact checks, regression coverage, and cross-platform CI.
justinblethrow-cloud
force-pushed
the
upstream/consolidated-dumi
branch
from
July 15, 2026 04:27
b4f0a2a to
f8d4a37
Compare
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.
Summary
This PR consolidates the generally useful correctness, I/O-safety, and
performance work developed and validated in the
dUMI maintained fork into one
upstream submission.
It:
@siddharthab's authorship, so paired mode does not reopen an indexed BAM at
every reference change;
SAM/BAM input;
N, missing/short-UMI, edit-bound, overflow, equality, anddeterministic-tie behavior;
scheduling in dense parallel components;
NgramBKTreeoverhead;
writer, and paired-mate cleanup/recovery;
Linux/macOS Java 11/21 CI.
Closes #31.
Reviewable commit series
The PR remains one submission, with changes divided by concern:
hardening;
Streaming mode
A new option controls the fast path:
offremains the upstream default and preserves the existing path andoutput ordering.
autostreams compatible input and safely restarts through the existingpath if coordinate-order metadata proves inaccurate or the clipping window
is insufficient.
onrequires streaming and fails explicitly when its contract is not met.Streaming is eligible only for coordinate-declared, actually
coordinate-ordered, single-end SAM/BAM input using sequential algorithms and
data structures with cluster tracking disabled. Paired, parallel, tagging,
two-pass, FASTQ, and declared-unsorted inputs retain the existing paths.
The implementation retains alignment groups only within the active coordinate
window. Positive-strand unclipped starts use a conservative 10,000-base lag
window, configurable with
-Dumicollapse.streaming.positiveLag=...; unsafe inputs fall back inautomode or fail in
onmode.Because completed groups can be emitted outside coordinate order, streaming
output is correctly declared
SO:unsorted. Record content is checked againstthe existing path, but record order is not guaranteed to match.
Correctness and failure safety
Nstate through cloning, mutation, equality,hashing, ordering, and generated neighbor keys.
different UMI; custom separators are treated literally.
malformed input fail explicitly.
intentionally change which equal-scoring representative is selected;
anyremains arbitrary.a node once per query.
and promotes it only after successful processing and close. Existing output
survives a failure, and input/output aliases and hard links are rejected.
sequential fallback for SAM, unindexed BAM, and unresolved cross-reference
mates, with exact read-name/reference/start matching.
Performance work
constructing a clustering structure for the common one-UMI case while
preserving custom Java extension semantics.
NgramBKTreeuses packed primitive keys where representable, falls backsafely outside the packing range, and caps initial capacity by the finite
five-base n-gram universe.
with failure-safe resource ownership and non-indexed recovery.
The maintained fork's
sealed synthetic benchmark
compares canonical upstream
efeab35with the corresponding dUMI productionimplementation at
2995329. All compared outputs passed record-multiset andheader-semantic gates. Selected median matched-repetition results:
The small paired fixture was 8.9% slower, so this is not presented as a
universal speedup. These are fixed-seed synthetic scaling results on one
non-exclusive host, not production-throughput guarantees. The large paired
gain is principally attributable to #32.
Dependencies and build
classpaths are derived from
dependencies.lock; unexpected lock syntax,duplicate names, unsafe paths, non-HTTPS URLs, and checksum mismatches fail.
--release 11 -Xlint:all -Werror.bytes, manifest identity, Java target, and exclusion of test classes.
thread stack.
The tracked JAR is retained to respect the current upstream distribution
policy. The maintained fork's release packaging, SBOM, provenance,
reproducible-archive framework, benchmarks, profiles, and presentation
materials are deliberately not included here.
Validation
The exact PR head passed:
./scripts/check.shruns on Java 11 and Java 21;git diff --checkand identity/private-collateral scans; andfindings.
The regression suite covers sequential and parallel data structures,
ambiguous/generated UMIs, thresholds and overflow, deterministic
representatives, deep graph/BK-tree traversal, dense scheduling, n-gram
differential parity, singleton/custom-extension behavior, SAM/BAM streaming
compatibility and fallback, FASTQ/CLI transactions, malformed-input cleanup,
same-file/hard-link rejection, and paired recovery from indexed BAM, unindexed
BAM, and SAM.
The public
v2.0.0 validation record
and release
retain the full fork-level evidence.
Compatibility and intentional behavior changes
fails early.
exactly equal, without changing the intended cluster membership.
this PR.
Prior work and attribution
its original authorship. It addresses the repeated indexed-BAM-open cost
reported in Very slow paired reads mode for transcriptome #31.
Intervalallocation as anNgramBKTreehotspot. This implementation addresses the same hotspot withpacked keys that include sequence and interval bounds, a safe fallback
outside representable limits, no new dependency, and differential testing
against
Naive. Credit to @0jvh398j for identifying and reporting thehotspot.