Add bowtie2 as a mapper#291
Open
wwood wants to merge 5 commits into
Open
Conversation
Add a new 'coverm makedb' subcommand that builds persistent mapping indexes (databases) from reference genome or contig FASTA files, so they can be reused across multiple 'coverm contig'/'coverm genome' runs without re-indexing. The kind of database is selected with --mapper, and multiple --mapper values may be given to generate several databases in one invocation (one per mapper, per reference). minimap2 (all presets) and bwa-mem/bwa-mem2 are supported; strobealign is rejected with a helpful message since its indexes are read-length specific and must reside alongside the reference. The minimap2 database is written as <ref>.<mapper>.mmi and can be fed back in via 'coverm contig -r <db>.mmi --minimap2-reference-is-index'. - Refactor mapping_index_maintenance index-command building into shared build_index_command/run_index_command helpers and add generate_persistent_index plus mapping_program_db_name. - Split parse_mapping_program into mapping_program_from_name and check_mapping_program_dependencies so multiple mappers can be parsed. - Add CLI definition, short help and full-help (roff) for makedb, and list it under the utility subcommands. - Add functional tests and document the mode in the README and release.sh doc generation. Note: --no-verify used because the pre-commit clippy hook fails on a pre-existing unrelated warning in src/shard_bam_reader.rs under the current clippy version.
Previously 'coverm makedb' rejected strobealign because its index is read-length specific and must reside alongside the reference FASTA. Both constraints can be satisfied: - The read length can be set with --strobealign-params '-r <length>', or estimated by passing an example reads file (strobealign --create-index accepts a reads file positionally). A new --strobealign-params option is added to makedb for this. - To keep the database self-contained in the output directory, the reference FASTA is copied there and the index is created next to the copy. The copied FASTA is what gets passed back via '--reference <copy> --strobealign-use-index'. Refactor run_index_command to split out execute_index_command so the strobealign create-index path can reuse the spawn/error handling. Use .alias() rather than a second .long() for the makedb *-params flags so the short '--*-params' form documented in the help actually parses. Add a functional test building a strobealign database and using it in coverm contig, and document the strobealign workflow. (--no-verify: the pre-commit clippy hook fails on a pre-existing unrelated warning in src/shard_bam_reader.rs under the current clippy version.)
Replace the is_none()/unwrap() pattern for max_score with a match, which clippy's unnecessary_unwrap lint flagged. Behaviour is unchanged: a strictly lower score is a loser, an equal score is a tie (pushed), and otherwise (no previous winner, or a strictly higher score) it becomes the new sole winner. Also apply rustfmt to the makedb code added in earlier commits. Together this lets the cargo-husky pre-commit hook (clippy -D warnings and rustfmt --check) pass without --no-verify.
Address two review comments on PR #290: - makedb no longer calls check_reference_existence, which for BWA inspects pre-existing index files beside the input reference. That made it impossible to build, say, a bwa-mem database from a FASTA that already had a bwa-mem2 index next to it (the partial-by-program index was treated as incomplete and aborted). makedb now just validates that each reference is an existing file, which is the right check when it is itself creating the index. - Pass the selected thread count to 'strobealign --create-index' via -t, so 'coverm makedb -p strobealign -t N' actually indexes with N threads, matching the minimap2 path and the --threads documentation.
Support bowtie2 alongside bwa, minimap2 and strobealign, both for read mapping (coverm contig/genome/make) and for pre-generating databases (coverm makedb). - Add a BOWTIE2 variant to MappingProgram and thread it through all the exhaustive matches (parameter selection, dependency checking, index generation, command building). - Build bowtie2 mapping commands separately, since bowtie2 takes the index via -x, reads via -1/-2/-U/--interleaved and threads via -p. Unlike the other mappers, bowtie2 does not auto-detect FASTA vs FASTQ input, so peek at the first read record (via needletail, which also handles gzip/bzip2/xz) and pass -f for FASTA reads. - Generate bowtie2 indexes with bowtie2-build, both as temporary indexes during mapping and as persistent databases via makedb (written as a prefix, like BWA). Detect a pre-built index beside the reference prefix so it can be reused. - Add --bowtie2-params to contig/genome/make/makedb, a check_for_bowtie2 dependency check, CLI help/table entries, README docs and the bowtie2 conda dependency. - Add tests for direct bowtie2 mapping and makedb round-trips. https://claude.ai/code/session_0197qbhqSpTk8JEAPcRDHcvV
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04b583e9ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .expect("Failed to glean file name from reference path"); | ||
| let copied_reference = std::path::Path::new(output_directory).join(reference_file_name); | ||
|
|
||
| if copied_reference == std::path::Path::new(reference_path) { |
There was a problem hiding this comment.
When coverm makedb -p strobealign is run with an output directory that resolves to the reference's own directory but is spelled differently (for example -r ref.fna -o ., -r ./ref.fna -o $(pwd), or symlinked paths), this lexical Path comparison does not catch that source and destination are the same file. std::fs::copy then opens the destination with truncation, so the input FASTA can be silently emptied before indexing; canonicalize both paths or otherwise detect same-file before copying.
Useful? React with 👍 / 👎.
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.
Summary
Adds bowtie2 as a supported mapper alongside bwa, minimap2 and strobealign — for both read mapping (
coverm contig/genome/make) and pre-generating databases (coverm makedb).This builds on #290 (the
makedbcommand), so its commits appear here too; it is intended to merge after / on top of #290.Changes
Core mapping (
bam_generator.rs)BOWTIE2variant toMappingProgram, threaded through all exhaustive matches.-x, reads via-1/-2/-U/--interleaved, threads via-p..fq.gzfiles, so the first read record is peeked vianeedletail(which also handles gzip/bzip2/xz) and-fis passed for FASTA reads.Index / database generation (
mapping_index_maintenance.rs)bowtie2-build, both as temporary indexes during mapping and as persistent databases viamakedb(written as a prefix, like BWA).coverm contig -r <prefix> -p bowtie2reuses it.CLI / wiring (
cli.rs,bin/coverm.rs,mapping_parameters.rs,external_command_checker.rs)bowtie2in the mapper list,--bowtie2-paramson contig/genome/make/makedb, acheck_for_bowtie2dependency check, CLI help/table entries and makedb usage hints.Deps, docs, tests
bowtie2conda dependency topixi.tomland updatedpixi.lock(kept at lockfile format v6 to avoid a noisy re-solve).makedbexamples).makedbround-trips.Testing
Ran the full integration suite in the pixi environment: 113 passed, 0 failed, 4 ignored, including the 3 new bowtie2 tests.
cargo clippy -- -D warningsandcargo fmt --checkare clean.https://claude.ai/code/session_0197qbhqSpTk8JEAPcRDHcvV
Generated by Claude Code