Repository navigation
Shuffle gen-reads FASTQ output by default (shuffle_fastq, #870) - #875
Merged
Merged
Conversation
gen-reads wrote reads in genome order, which no sequencer produces; on a 21x T2T-CHM13 simulation that made bwa-mem2 need 166 GB against 90 GB for the same reads shuffled. The combine step now does a two-pass external shuffle: records (pairs together) are scattered at random into bucket files, each bucket is permuted in memory, and the buckets are concatenated. Memory is bounded by bucket size and a 4 GiB pass-2 budget; temp disk stays near one copy because inputs are deleted as read. The shuffle draws from its own child of the run RNG, so reads, the truth VCF and the golden BAM are unchanged, and output is byte-identical across thread counts. shuffle_fastq: false restores genome order. Co-Authored-By: Claude <noreply@anthropic.com>
…astq # Conflicts: # docs-site/src/gen-reads/fastq-shuffling.md
…reads (#870) Review of #875. The scatter held one writer per bucket per mate, 2 * 256 descriptors at the cap, which fails under common 256/512 limits. Buckets now accumulate in memory and leave as gzip members appended to their file, which is closed again, so at most one bucket file is open at any bucket count. MAX_BUCKETS rises to 512 (the cap is now buffer memory: 256 MiB). A test re-runs itself at 512 paired buckets under ulimit -n 48; on the old code it failed with "Too many open files" at bucket 22. Thread-count determinism is now asserted on the raw gzip bytes as well as the decompressed records, in the unit and integration tests; an empty-member-on-multithread mutant passes the record check and fails the byte check. BlockGzWriter::with_level is dropped as unused. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
Both review points addressed in 7b4e54c. File descriptors at the bucket cap.
Thread-count determinism on raw bytes.
Cost after the change (CHM13 chr21, 20×, ~3M pairs, 8 threads):
|
This was referenced Nov 5, 2026
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.
Closes #870. Supersedes #873, which can be closed: this PR rewrites the same docs page and keeps its measured numbers.
Semver: MINOR. This adds a config key, and seeded runs now write FASTQ records in a different order with the same content. Existing configs still load and run. Treat it as MAJOR instead if record order counts as public API; the release notes need an upgrade line either way.
Why
gen-reads wrote reads in genome order, which no sequencer produces. On a 21x T2T-CHM13 simulation, bwa-mem2 peaked at 166 GB on the reads as written and 90 GB on the same reads shuffled, with matching flagstat (#870, jobs 23383724 and 23394381). The old in-memory shuffle was removed in v1.5.0 because it held every read at once.
Change
New key
shuffle_fastq, defaulttrue;falserestores genome order byte for byte.eidolon-core/src/file_tools/fastq_shuffle.rsis a two-pass external shuffle in the combine step:Pass 2 runs only as many buckets at once as fit in 4 GiB, judged by the largest bucket. Bucket files use libdeflate's fastest level (
BlockGzWriter::with_level).Determinism: the scatter draws from one RNG child of the run (
SHUFFLE_RNG_CHILD), and each bucket's permutation from a child of that. Thread scheduling only changes which bucket runs when.Isolation: reads, the truth VCF and the golden BAM do not depend on the shuffle.
Tests:
determinism.rs::ordinary_reads_are_emitted_in_reference_file_ordernow pins theshuffle_fastq: falsepath. That order is also the input order the shuffle's determinism rests on.GenReadsConfiggainsshuffle_fastq.Docs: the shuffling page is rewritten for the built-in shuffle (the seqkit recipe stays, for older output); the template documents the key; the parallel-processing page links to it.
Evidence
Unit tests (
fastq_shuffle.rs, 8):bucket_countwould always choose 1 and leave the scatter untested.Integration (
tests/fastq_shuffle_output.rs, gen-reads on H1N1):shuffle_fastq: false.Mutation checks (each confirmed applied; source restored and verified with cmp):
shuffle_fastq: falseNot catchable from output: sending every record to one bucket still yields a correct shuffle and only loses the memory bound.
bucket_countandshuffle_threadsare unit-tested instead.Cost (local, CHM13 chr21, 20×, ~3M pairs, 8 threads; off → on):
The workspace suite passes (65 test binaries,
--no-fail-fast), along with clippy-D warningsand fmt.Not verified
gen-cancer-reads) concatenate the normal and tumor passes, each now shuffled. The merged FASTQ is therefore unordered within each pass but not interleaved between passes.germline_e2e'sSHUFFLE=1becomes redundant, and nothing there depends on order.🤖 Generated with Claude Code