Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,13 @@ Uses standard double-dash flags (`--contig`, `--abund`, etc.). Single-dash compa
- `maxbin-rs filter` — filter contigs by minimum length
- `maxbin-rs seeds` — generate seed file from HMMER marker gene hits
- `maxbin-rs em` — run the Rust EM algorithm
- `maxbin-rs cpp-em` — run the original C++ EM via FFI (equivalence testing only)
- `maxbin-rs sam-to-abund` — compute abundance from a SAM file

The original C++ EM (used only for equivalence testing) now lives in a
separate crate: run it via `maxbin-rs-cpp-em` from `crates/maxbin-rs-equivalence`
(see below). It is no longer a `maxbin-rs` subcommand, so the core crate carries
no C/C++.

### Key source files (in `crates/maxbin-rs/src/`)

- `pipeline.rs` — orchestration for each subcommand
Expand All @@ -53,10 +57,30 @@ Uses standard double-dash flags (`--contig`, `--abund`, etc.). Single-dash compa
- `kmer_map.rs` — tetranucleotide frequency computation
- `abundance.rs` — abundance file parsing
- `fasta.rs` — FASTA reading/writing
- `original_ffi.rs` — C++ FFI bridge for equivalence testing against the original EM
- `external.rs` — shelling out to HMMER, Bowtie2, gene caller
- `profiler.rs` — timing instrumentation

The core crate has no `build.rs` and no C/C++ dependency, so it can eventually
compile to wasm.

### Equivalence crate (`crates/maxbin-rs-equivalence/`)

Holds everything that links the original MaxBin2 C++ for FFI-based equivalence
testing, kept out of the core crate:

- `build.rs` + `vendor/`: extract the upstream MaxBin2 C++ from the tarball
(`MAXBIN2_SRC_TARBALL`), apply `vendor/maxbin2-cpp-ffi.patch`, and compile it
as a static lib via `cc`.
- `src/original_ffi.rs`: Rust FFI bindings to that C++ (re-exported from the
crate's `lib.rs`).
- `src/bin/maxbin-rs-cpp-em.rs`: the `maxbin-rs-cpp-em` binary that runs the
original C++ EM via FFI (formerly the `cpp-em` subcommand).
- `tests/`: the FFI equivalence tests and the FFI-comparing proptests. They
exercise the core crate's public API and compare it against the C++. The
`divergent-em` fixture lives here too.

The crate depends on `maxbin-rs` as a path dependency.

### Parallelism

Both the inner abundance-file loop and the outer contig loop in the EM E-step are parallelized with Rayon, controlled by `--thread`.
Expand All @@ -74,7 +98,7 @@ Outputs: one FASTA per bin + summary statistics + `.log` file.

## Testing Strategy

- **Equivalence tests** compare Rust output against original MaxBin2 on the same input, including reproducing known bugs. The C++ EM is available via FFI (`cpp-em` subcommand) for component-level comparison.
- **Equivalence tests** compare Rust output against original MaxBin2 on the same input, including reproducing known bugs. The C++ EM is available via FFI in the `maxbin-rs-equivalence` crate (the `maxbin-rs-cpp-em` binary) for component-level comparison.
- **Property tests** (proptest) cover parsing, distance computation, EM numerics, and sorting.
- **Benchmarks** via `cargo nextest run --cargo-profile bench bench_components -- --ignored`.
- When an equivalence test reveals a bug in the original, **document it** (in TODO.md and/or inline) but still reproduce the buggy behavior in the v0.1.x code path.
Expand Down
13 changes: 11 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[workspace]
resolver = "3"
members = ["crates/maxbin-rs"]
members = ["crates/maxbin-rs", "crates/maxbin-rs-equivalence"]

# Profiles are only honored at the workspace root, so the benchmark profile
# that used to live in the crate manifest moves here.
Expand Down
16 changes: 16 additions & 0 deletions crates/maxbin-rs-equivalence/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "maxbin-rs-equivalence"
version = "0.4.0"
edition = "2024"
publish = false

[dependencies]
clap = { version = "4.6.1", features = ["derive"] }
maxbin-rs = { path = "../maxbin-rs" }

[build-dependencies]
cc = "1.2.60"

[dev-dependencies]
flate2 = "1.1.9"
proptest = "1.11.0"
File renamed without changes.
48 changes: 48 additions & 0 deletions crates/maxbin-rs-equivalence/src/bin/maxbin-rs-cpp-em.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//! `maxbin-rs-cpp-em`: run the original MaxBin2 C++ EM via FFI.
//!
//! This binary exists only for equivalence testing and benchmarking against
//! the original implementation. The core `maxbin-rs` crate no longer carries
//! the C++ FFI, so this lives in the equivalence crate.
//!
//! The C++ EManager handles its own data loading, EM, classification, and
//! file writing internally, so the whole thing is timed as one block.
use std::path::PathBuf;

use clap::Parser;
use maxbin_rs_equivalence::original_ffi::OriginalEManager;

#[derive(Parser)]
#[command(
name = "maxbin-rs-cpp-em",
about = "Run the original MaxBin2 C++ EM via FFI (equivalence testing)."
)]
struct Args {
#[arg(long)]
contig: PathBuf,
#[arg(long)]
abund: PathBuf,
#[arg(long)]
seed: PathBuf,
#[arg(long)]
out: String,
#[arg(long, default_value_t = 1)]
thread: usize,
}

fn main() {
let args = Args::parse();

eprintln!("Running original C++ EM via FFI...");
eprintln!(" contigs: {}", args.contig.display());
eprintln!(" abund: {}", args.abund.display());
eprintln!(" seed: {}", args.seed.display());
eprintln!(" threads: {}", args.thread);

let t = std::time::Instant::now();
let em = OriginalEManager::new(&args.contig, &args.abund, &args.out);
em.set_thread_num(args.thread as i32);
let result = em.run(&args.seed);
let secs = t.elapsed().as_secs_f64();

eprintln!("C++ EM completed in {secs:.1}s (returned: {result})");
}
14 changes: 14 additions & 0 deletions crates/maxbin-rs-equivalence/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//! Equivalence-testing harness for maxbin-rs.
//!
//! This crate links the original MaxBin2 C++ code as a static library and
//! exposes Rust FFI bindings to it (`original_ffi`). It exists so the core
//! `maxbin-rs` crate can stay free of any C/C++ toolchain dependency (and
//! eventually compile to wasm), while the bit-for-bit equivalence tests
//! against the original implementation keep running here.
//!
//! The C++ sources are extracted from the upstream MaxBin2 tarball and
//! patched for FFI at build time (see `build.rs` and `vendor/`). The
//! `MAXBIN2_SRC_TARBALL` environment variable points at that tarball; the
//! Nix devshell sets it automatically.

pub mod original_ffi;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/// FFI bindings to the original MaxBin2 C++ code.
/// Used only for equivalence testing — not compiled into release builds.
/// Used only for equivalence testing, inside the maxbin-rs-equivalence crate.
use std::ffi::{CStr, CString};
use std::os::raw::c_char;

Expand Down
43 changes: 43 additions & 0 deletions crates/maxbin-rs-equivalence/tests/abundance_ffi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//! FFI equivalence test for the abundance-file parser.
//!
//! Compares `maxbin_rs::abundance` against the original MaxBin2 C++
//! AbundanceLoader. Previously an in-source test in `abundance.rs`.
use maxbin_rs::abundance::parse_file;
use maxbin_rs_equivalence::original_ffi::OriginalAbundanceLoader;

#[test]
fn equivalence_with_original() {
// Writes a temp abundance file, parses with both Rust and C++ FFI,
// compares field by field.
use std::io::Write;

let test_data = "contig_1\t10.5\ncontig_2\t0.00001\ncontig_3\t100.0\ncontig_4 0.5\n";

let tmp_path = std::env::temp_dir().join("maxbin_rs_test_abund.txt");
{
let mut f = std::fs::File::create(&tmp_path).unwrap();
f.write_all(test_data.as_bytes()).unwrap();
}

// Rust
let rust_records = parse_file(&tmp_path).unwrap();

// Original C++
let original = OriginalAbundanceLoader::new(&tmp_path);

assert_eq!(rust_records.len(), original.num_records() as usize);
assert!(original.is_parse_success());

for (i, rec) in rust_records.iter().enumerate() {
let orig_abund = original.abundance_by_index(i as i32);
assert!(
(rec.abundance - orig_abund).abs() < 1e-10,
"abundance mismatch at record {i} (header: {}): rust={} original={}",
rec.header,
rec.abundance,
orig_abund,
);
}

std::fs::remove_file(&tmp_path).ok();
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ fn bench_normal_distribution() -> (f64, f64) {
let inputs: Vec<f64> = (0..1000).map(|i| i as f64 * 0.001).collect();

let cpp_count = count_ops(BENCH_DURATION, || {
let nd = maxbin_rs::original_ffi::OriginalNormalDistribution::new(mean, std_dev);
let nd =
maxbin_rs_equivalence::original_ffi::OriginalNormalDistribution::new(mean, std_dev);
for &x in &inputs {
std::hint::black_box(nd.prob(x));
}
Expand All @@ -92,7 +93,7 @@ fn bench_quicksort() -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let mut arr = base.clone();
let mut idx: Vec<i32> = (0..arr.len() as i32).collect();
maxbin_rs::original_ffi::original_quicksort(&mut arr, Some(&mut idx));
maxbin_rs_equivalence::original_ffi::original_quicksort(&mut arr, Some(&mut idx));
std::hint::black_box(&arr);
});

Expand Down Expand Up @@ -125,7 +126,7 @@ fn bench_kmermap_lookup() -> (f64, f64) {
};

let cpp_count = count_ops(BENCH_DURATION, || {
let km = maxbin_rs::original_ffi::OriginalKmerMap::new(4, true);
let km = maxbin_rs_equivalence::original_ffi::OriginalKmerMap::new(4, true);
for kmer in &kmers {
std::hint::black_box(km.get_mapping(kmer));
}
Expand All @@ -145,10 +146,10 @@ fn bench_kmermap_lookup() -> (f64, f64) {

fn bench_profiler(seqs: &[Vec<u8>]) -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let km = maxbin_rs::original_ffi::OriginalKmerMap::new(4, true);
let km = maxbin_rs_equivalence::original_ffi::OriginalKmerMap::new(4, true);
for seq in seqs {
let s = std::str::from_utf8(seq).unwrap();
let prof = maxbin_rs::original_ffi::OriginalProfiler::new(4, s, &km);
let prof = maxbin_rs_equivalence::original_ffi::OriginalProfiler::new(4, s, &km);
std::hint::black_box(prof.get_profile(136));
}
});
Expand All @@ -168,7 +169,7 @@ fn bench_profiler(seqs: &[Vec<u8>]) -> (f64, f64) {

fn bench_euc_dist(profiles: &[(Vec<f64>, Vec<f64>)]) -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let cpp = maxbin_rs::original_ffi::OriginalEucDist::new(4);
let cpp = maxbin_rs_equivalence::original_ffi::OriginalEucDist::new(4);
for (p1, p2) in profiles {
std::hint::black_box(cpp.get_dist_profile(p1, p2));
}
Expand All @@ -187,7 +188,7 @@ fn bench_euc_dist(profiles: &[(Vec<f64>, Vec<f64>)]) -> (f64, f64) {

fn bench_spearman_dist(profiles: &[(Vec<f64>, Vec<f64>)]) -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let cpp = maxbin_rs::original_ffi::OriginalSpearmanDist::new(4);
let cpp = maxbin_rs_equivalence::original_ffi::OriginalSpearmanDist::new(4);
for (p1, p2) in profiles {
std::hint::black_box(cpp.get_dist_profile(p1, p2));
}
Expand All @@ -207,7 +208,7 @@ fn bench_spearman_dist(profiles: &[(Vec<f64>, Vec<f64>)]) -> (f64, f64) {

fn bench_fasta_parse(path: &std::path::Path, raw_bytes: &[u8]) -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let reader = maxbin_rs::original_ffi::OriginalFastaReader::new(path);
let reader = maxbin_rs_equivalence::original_ffi::OriginalFastaReader::new(path);
std::hint::black_box(reader.num_records());
});

Expand All @@ -223,7 +224,7 @@ fn bench_fasta_parse(path: &std::path::Path, raw_bytes: &[u8]) -> (f64, f64) {

fn bench_abundance_parse(path: &std::path::Path, raw_bytes: &[u8]) -> (f64, f64) {
let cpp_count = count_ops(BENCH_DURATION, || {
let loader = maxbin_rs::original_ffi::OriginalAbundanceLoader::new(path);
let loader = maxbin_rs_equivalence::original_ffi::OriginalAbundanceLoader::new(path);
std::hint::black_box(loader.num_records());
});

Expand Down
Loading
Loading