Repository navigation
Conversation
… CLIs 612 tests in scripts/test/test_*.py covering input formats and read preprocessing, alignment, scoring, reporting and effort options, paired-end semantics, output and SAM options, the wrapper scripts, and index building and inspection. Expected values are derived from reads cut at known positions from fixed random references, using the MANUAL's scoring rules; tests that pin current output say so in their docstring. Run after building, from the repository root: make -j4 allall python3 -m pytest scripts/test Optional tools (zstd, lz4, bzip2, samtools) and the -debug/-sanitized binaries are skipped when absent. BOWTIE2_BIN_DIR points the suite at binaries built elsewhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bowtie2-align is 1.7x to 4.3x faster on every input timed (end-to-end and local, single- and paired-end, -p 1 to -p 16), except a 12-read input at -p 1, where index loading dominates. Its output is byte for byte the same as before. Peak memory is unchanged, except that with --mm a small run touches more of the mapped index. Dynamic programming (aligner_swsse_ee_u8.cpp, aligner_swsse_loc_u8.cpp, aligner_swsse.cpp, aligner_sw*.cpp): - The 8-bit end-to-end fill computes the vertical-gap term with an exact prefix scan across lanes instead of the lazy-F loop. Both 8-bit fills have a shorter dependency chain from one column to the next. Reads of up to 128 rows are filled as two interleaved chains. - The query profile is built by table lookup. The 8-bit backtraces clear only the intervals they wrote, not whole matrices. - Local mode gathers its candidate cells with vector instructions, sorts them by counting, tests domination vectorized, and backtraces without a frame stack. FM index and seeds (aligner_seed.cpp, group_walk.h, bt2_idx.*): - Occurrence counts in every LF step are branch-free, and the large index arrays are 64-byte aligned. - Exact and one-mismatch seed searches are batched across the reads of an input batch and across both strands. Independent walks are stepped in turn, with each walk's next BWT side prefetched, so memory waits overlap. - Seed instantiation is redone only for inputs that changed. Seed-hit ranking and sampling no longer scan lists linearly or sort per call. - SA resolution runs single-row walks ahead of their use, and walks large BW ranges a 64-bit BWT word at a time. Sampled SA elements are built on first use. Every random draw is unchanged. - joinedToTextOff uses a bucket table built at index load instead of a binary search over the reference fragments. Elsewhere: RedundantAlns keeps per-read runs along diagonals, BtBranchTracer resets lazily, reference windows are unpacked straight to masks, and the output queue and SAM writer do less work per record. Index loading (index_load.h): at -p > 1, the four large arrays (.4 file, forward and mirror BWT, SA sample) are read in 64 MB pieces by up to four helper threads and the main thread. Read errors print the same messages as the serial load. The load stays serial at -p 1, and with --mm, --shmem, --verbose, -t or --sanity. AVX2 dispatch (bowtie_main.cpp, Makefile): the dispatch tested AVX2 in CPUID leaf 1, so bowtie2-align-*-v256 ran only on CPUs where an unrelated bit was set. It now requires every x86-64-v3 feature (CPUID leaves 1, 7 and 0x80000001) and OS support for the AVX registers (XCR0). The -v256 build compiles with -ffp-contract=off, so its floating-point results, --score-min and MAPQ, are rounded as in the 128-bit build. No OS-specific calls are added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi Ben, hope you're doing well. The above message was written by Claude, but this comment is written by me (though based on my MACS3 PR). I have been developing a tool, called [auto-optimize], which basically applies the auto-research loop to code to improve speed and memory usage of code, but with extensive robustness checks to try to ensure nothing subtly breaks. I have been applying [auto-optimize] across the genomics ecosystem, starting with my own tools but now extending to other important ecosystem tools, with the hope of making things broadly better. Most recently, an auto-optimization of MACS3 was merged into that repository: macs3-project/MACS#756, and I've been impressed with other findings, such as this 20 speedup on my FIMO implementation: jmschrei/memesuite-lite#22. In an effort to be more robust, the process first augments the unit tests to make sure that nothing breaks during the search. See #535 (feel free to merge that independently of any choice you make here). Any iteration in our loop that broke any unit test was automatically discarded, and we used a set of external validations that were not directly optimized against each iteration to make sure we got identical results. At the end, we ran several thousand mapping jobs across assays and data type (single-end vs paired-end, read length, etc), covering every command-line flag, combinations, etc. Importantly, we also created a set of malformed data (both the reads and the genome) to make sure that not only did you get identical outputs on good data, but that it failed the same way on bad data so that we didn't just get rid of safeties that were necessary but slow. If you look at the results below, you'll see that there were dozens of small improvements that, together, led to the large improvements in speed. The big dip at the start was the end-to-end 8-bit DP fill. The purple run at the end was me reverting one change that required massive pages and may have caused issues on some architectures, which was probably a barrier to adoption.
In an effort to be consistent with https://rewrites.bio/, I'd like this to be merged into your repository so you can continue to get credit for all the work you've done and work you've done for the field (though I don't think that's really in question...). However, as a maintainer myself, I understand being skeptical of merging a PR of this size, particularly when the submitter can't explain everything in it (a consequence of so many changes being made at a low level). If you feel uncomfortable with it, we're happy to host a new repository that would point all credits and citations back to you but be a stand-alone entity. We're happy to share any logs generated during the process with you as well, or anything else you might need. |
In the 8-bit end-to-end and local backtraces, the step from vector row 0 to the previous element of the last vector row is btUpWrap = (colstride - ROWSTRIDE) * NBYTES_PER_REG - 1, a size_t. When the matrix has one vector row (reads of up to 16 bases with 16-byte vectors), it is (size_t)-1, and btcur + btUpWrap wraps around the address space to btcur - 1. The address is the intended one, but the wrap is undefined behaviour, which clang's -fsanitize=pointer-overflow reports; the sanitized build in the macOS CI job aborted on it. Casting to ptrdiff_t at the addition makes the step a plain -1. With gcc 13 on x86-64, the release aligners' machine code is identical with and without the cast. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This reply was written with Claude Code. The macOS "Simple Tests" failure was a real bug in this PR, fixed in 9bdab09. In the rewritten 8-bit backtraces ( The fix casts the offset to - uint8_t * const btup = btwrap ? btcur + btUpWrap : btcur - btUp;
+ uint8_t * const btup = btwrap ? btcur + (ptrdiff_t)btUpWrap : btcur - btUp; // (size_t)-1 when nvecrow_ is 1With GCC 13 on x86-64, the release aligner binaries' machine code is byte-identical with and without the cast, so speed and output are unchanged. With clang 19 on Linux, 🤖 Generated with Claude Code |
|
@jmschrei why make one massive PR? You could break this up into more manageable chunks, or ask Claude to do it for you. I think, for example, all of the e2e 8-bit fill optimisations (#1, #5, #9 second point, #25) can be nicely put into their own self-contained PR and they achieve a notable portion of the speedup. This is also important for the stacks of small improvements, as I found that a couple of them (#23 and #33) were regressions on my hardware and a few more were not measurable improvements. I just happened upon this PR so please lmk if the hardware you are testing on is particularly relevant compared to my consumer CPU, or any other misconceptions I may have. |
|
Hi @jw1912, thanks for commenting. The output, as provided, is what was discovered by the auto-optimization algorithm through its search process. I'd be happy to split this into smaller chunks if that's what the core developers here are interested in. I'm also happy for them to cherry-pick any individual or set of ideas into cleaner PRs. With respect to the potential for regressions: the auto-optimization search process is supposed to only consider ideas that work on all major operating systems and not to rely on OS-specific tools or flags, but admittedly that is prompt-based so has the potential for error. It is possible that some would not transfer across machines -- it is also possible that some only become improvements once other portions of the code have been improved. In general, I'm happy to redo this process with whichever sorts of constraints would make the most sense (e.g., increasing the noise band so only big improvements get in, avoiding certain portions of the code/flags, etc). I was more interested in starting the conversation about the best way to integrate AI-discovered speedups into these code-bases, rather than saying that this is the only way to move forward. |
|
@ch4rr0 and I will be taking a look at this soon. When I first read through the PR, I definitely left with the impression that it should be split up, but we'll comment more thoroughly when we've had a chance to digest and discuss. |
|
Sounds good, thanks for taking the time to discuss. Let me know if I can be helpful in any manner. |
|
We are indeed inclined to break this up. As a sample, here's one piece: #542. @jmschrei -- you are co-author on all but one of those commits in PR542. That last commit is a small fix to avoid an assert in the debug binary. Let us know if you have any comments on this. I intend to try to keep going this way; it's not clear to me yet whether we'll end up using everything from #536; but we'll keep plugging away and taking chunks. |
|
Sounds great! Thanks for managing the process. Take/discard whatever you'd like. |
|
Here's the next one: #544 |
Five seed-search and index speedups from #536, and a debug-check fix
Three dynamic-programming speedups from #536, with tests and a fix-up guard

This optimization was found with auto-optimize, a tool developed by the Programmable Genomics Laboratory at UMass Chan Medical School: it speeds up
bowtie2-alignby 1.7x to 4.3x on every input timed except a 12-read one at-p 1(1.0x, where index loading dominates), with output byte-identical tomaster.On the four main cases at
-p 4(ATAC-seq, ChIP-seq, CUT&RUN and DNase-seq against hg38), the mean wall time falls from 19.70 s to 6.49 s (3.04x). Most of the gain comes from three things: rewriting the 8-bit dynamic-programming fills, batching and overlapping FM-index walks across reads, and loading the index in parallel. Peak memory is unchanged apart from small--mmruns (see Memory), and no OS-specific calls are added.It builds on #535, which adds a pytest suite and changes no source. The first commit here is #535's. This pull request's own change is the second commit: 37 files, +7,472 / -1,815. Once #535 is merged, only the second commit remains.
Changes
In the order they were made:
aligner_swsse_ee_u8.cpp): an exact prefix scan across lanes replaces the lazy-F loop, which took about 18 passes per column.bowtie_main.cpp,Makefile): the dispatch tested AVX2 in CPUID leaf 1 instead of leaf 7, sobowtie2-align-*-v256ran only on CPUs where an unrelated bit happened to be set. It now requires every x86-64-v3 feature (CPUID leaves 1, 7 and 0x80000001) and that the OS saves the AVX registers (XCR0). The-v256build compiles with-ffp-contract=off, so its floating-point results, and with them--score-minand MAPQ, match the 128-bit build. Short reads get a 128-bit fill path, where the 256-bit fill was slower.std::fill_n, newPList::addFillUpToinds.h) instead of with oneadd()call per element.masterzeroes 14.4 GB per 40,000 ATAC-seq pairs.RedundantAlns: keeps each read's cells as runs along diagonals instead of a set of cells.index_load.h): at-p > 1, the four large index arrays load concurrently on up to four helper threads. File errors are reported at the same point and with the same messages as the serial load.BtBranchTracer: resets lazily, only when the checkpointed backtracer runs.ftablookups are prefetched one round early, and a sweep restart's first LF step moves to the next turn.joinedToTextOff: uses a bucket table built at index load instead of a binary search over the reference fragments.Tests and CI
The pytest suite from #535 passes on this branch: 611 passed, 1 skipped. The Linux jobs of the repository's two workflows were run locally on this commit, as the workflow files write them, each in a fresh clone:
make clean; make -j4 allall; make simple-testSSE_AVX2=0make clean; make -j4 allall; NUM_CORES=1 make random-testSSE_AVX2=0python3 -m pytest scripts/teston both buildsThe macOS jobs could not be run here.
Behaviour changes
There is one. On CPUs where the old dispatch failed to detect AVX2,
bowtie2-align-sand-lnow run their-v256builds, and with-ffp-contract=offthose give the same output as the 128-bit builds. On CPUs where the old dispatch did select-v256, fused multiply-adds could change MAPQ relative to the 128-bit build. We measured that in 7,886 of 200,000 24-bp reads with a custom--score-min. Users on such CPUs therefore now get the 128-bit build's output. With default settings we saw no such difference: 221 of 221 jobs were byte-identical againstmaster's own-v256binary.Usage benchmarks
Machine: AMD EPYC 9575F, 2 sockets x 64 cores (256 hardware threads, 2 NUMA nodes), 1 TB RAM, Ubuntu 24.04.5, Linux 7.0, GCC 13.3.0, Perl 5.38.2. Both builds use the repository's
make.Inputs: no synthetic data. Every input is public sequencing data, aligned against hg38 (UCSC, 455 sequences) indexed by
master'sbowtie2-build.--end-to-end --very-sensitive --no-mixed --no-discordant -I 10 -X 700.How it was timed:
numactl --cpunodebind=0 --membind=0), and started only when at most two processes were runnable on the whole machine./usr/bin/time %M).@PGand sorting records when-p > 1without--reorder.master(s)masterpeak (MB)-X 2000-X 2000-X 2000-X 2000-X 2000--local--local--local--local--local--very-fast--very-sensitive--local--very-sensitive-local-k 5--reorder-k 10--local--mm--very-sensitiveOn the 105 full-depth experiments below, the summed wall time was 25.4 h for
masterand 7.9 h for this branch (3.20x). Those runs went two or three at a time beside other jobs, so they are not a controlled timing.How the output was checked
The usage benchmarks above check speed and memory. Separately, we ran
masterand this branch side by side on a collection of public sequencing data and hand-made malformed files:@PGlines are compared too.-p > 1without--reorder, record order varies from run to run inmasteritself, so those runs were compared as sorted records.Builds compared
mastermakemakemasterruns its 128-bit code (the dispatch bug) and this branch its-v256codemakemake SSE_AVX2=0bowtie2-align-*-sanitized, built with-fsanitize-recover-fsanitize=threadmaster-v256bowtie2-align-*-v256run directlymakemaster's AVX2 code, which its dispatch never selects heremakemasteragainmasterWhat ran
-p 1--local,--very-sensitive,--phred64--phred64on the Phred+64 files-p 3, 4, 8 and 16, with and without--reorder-c;-F; tab5/tab6; qseq; interleaved; stdin; comma-separated lists; mixed paired and unpaired; BAM, aligned and unaligned, with--align-paired-readsand--preserve-tags), trimming and read selection, presets, scoring, reporting (-k,-a), pairing, outputs (--un*/--al*,--met-file, SAM options), threads, indexes, and command-line errorsbowtie2,bowtie2-buildandbowtie2-inspectwith several values each, single-end, paired-end and-p 4 --reorderbowtie2-build,bowtie2-inspect+lines, unequal mate counts, zero-length reads, lowercase bases, odd quality characters, gzip CRC errors, BGZF without its EOF block-p 1,-p 4and-p 4 --mm-p 16 --reorder-p 4and-p 16, ordered and unordered output, damaged indexesmaster-v256--local)Results
master-v256mastervsmaster)"Identical failure" means both builds failed with byte-identical output. None of the differences comes from a change in what bowtie2 computes. Each cause is listed below with the evidence for it:
-p > 1, multithreadedbowtie2-buildlogs,--met-file,--met-stderr,--read-times,--non-deterministic, multithreaded--log-dpmasterdiffers from itself the same way (113 of the 129 controls)@PGcommand lines that record the wrapper's temporary file name (it carries the wrapper's PID, e.g. for bz2 input) or the output path of a--bamrun@PG--version, which prints the compiler options of the binary that runsbowtie2-inspecton an index built with-rprints an uninitialized valuebt2_idx.h:211vs:297)master; the records written before that are identical--localmodeApart from these, every exit code and output file is the same on valid input. On malformed input, both builds fail the same way in every case but one. When an index's
.3/.4files come from a different genome than its.1/.2files,mastersegfaults and the SSE2-only build of this branch loops. Behaviour there is undefined in both builds, because nothing checks that the files belong together.Sanitizers
master, and no longer triggered by this branch:SeedAligner::oneMmSearchon reads with N (aligner_seed.cpp, 545 commands);Ebwt::countUpTo(556 commands).memcpyinSStringExpandable::install;TIndexOffUloads with--mmon a large (.bt2l) index;+line;--preserve-tagsBAM input;OutFileBuf::flushBlocking()readsasyncData_.bufwithout the lock thatAsyncData::writeComplete()holds when it clears it.Threads at
-p 1The speedup does not come from extra threads:
-p 1, both builds create the same three threads, one worker plus the input reader and output writer thatmasteralready runs.stracecounted them on single-end ChIP-seq and on paired-end--localDNase-seq.-p 1, for example, it drops from 101.5 to 26.8 CPU-seconds.-p > 1: this branch adds up to four short-lived threads that read the index files in parallel. With-p 1,--mm,--shmem,--verbose,-tor--sanity, the index loads serially as before.Memory
Peak resident memory is unchanged: 3.3 to 3.8 GB for hg38 in every timed configuration, a ratio of this branch to
masterbetween 0.987 and 1.007.Across the 4,390 identical release commands that peaked above 500 MB without
--mm, the median ratio is 1.000 and the largest is 1.018. Seven used less than 95% ofmaster's memory, and all seven are-aruns.With
--mm, where the index is memory-mapped instead of copied, the resident set also counts the index pages a run touches. Over the 102--mmcommands the ratio ranged from 0.41 to 2.17:--local --mm -p 3. This branch peaked at 1.20 GB against 0.55 GB in three repeats, and took 0.31 s against 0.19 s.RssFile947-1,044 MB against 265-346 MB, sampled every 20 ms). Private memory was the same, 17-18 MB.--mm, both builds reach 3.3 GB, and this branch is 2.12x faster (table above).We did not establish which change touches more of the mapped index at start-up.
What this testing does not cover
bowtie2-buildoption combination, which was too slow; four ASan jobs that hang in both builds (-Fon zstd and xz FASTA).--score-minonmaster's-v256code: see Behaviour changes. Only the default-settings check above was run.Bugs in
masterfound along the wayWe reproduced 25 bugs in
master, and this branch behaves the same on each. We will file them as separate issues. Among them:-F k:<int>,i:<int>, the syntax--helpdocuments, yields 0 reads.--int-qualsaborts on any multi-base read.bowtie2-build --nodcand--threads 0die with SIGFPE, while the wrapper exits 0.--reorderat-p > 1hangs.-b, the--un/--alfiles contain raw BAM records.MC:Ztag makes output nondeterministic at-p > 1.🤖 Generated with Claude Code