diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..34f36e3 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,74 @@ +name: CI + +on: + # Every branch, not just `main`. A pull request opened from a fork by a + # first-time contributor does not run workflows until a maintainer approves + # them, so `pull_request` alone leaves reviewers with no check runs to look + # at. Building on push means the contributor's own fork produces evidence + # that can be linked from the PR. + push: + pull_request: + workflow_dispatch: + +env: + CARGO_TERM_COLOR: always + RUSTFLAGS: -D warnings + +jobs: + test: + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # macOS is aarch64/NEON, Ubuntu is x86_64/AVX2. The LCP kernel and the + # pooled external-memory bucket path are the parts that differ per + # platform, so both need to run. + os: [ubuntu-latest, macos-latest] + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + + # Debug catches the `debug_assert`s that guard the unchecked scatter in + # `radix.rs` and the buffer-length invariants in the merge kernel. + - name: Test (debug) + run: cargo test --all-targets + - name: Test (release) + run: cargo test --release --all-targets + - name: Doc tests + run: cargo test --doc + + lint: + name: fmt + clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + - uses: Swatinem/rust-cache@v2 + - run: cargo fmt --all --check + - run: cargo clippy --all-targets -- -D warnings + + msrv: + name: MSRV (1.89) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + # Pinned to the `rust-version` in Cargo.toml, which is set by the + # stabilised AVX-512 intrinsics the LCP fast path uses. + - uses: dtolnay/rust-toolchain@1.89 + - uses: Swatinem/rust-cache@v2 + - run: cargo check --all-targets + + docs: + name: rustdoc + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - run: cargo doc --no-deps + env: + RUSTDOCFLAGS: -D warnings diff --git a/Cargo.toml b/Cargo.toml index 12261dc..4827078 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,19 @@ categories = ["algorithms", "data-structures"] repository = "https://github.com/COMBINE-lab/caps-sa" readme = "README.md" +# The published benchmark numbers in `bench/` were taken with fat LTO and a +# single codegen unit. That configuration used to come from a parent workspace +# that no longer exists in this repo, so pin it here. A `[profile.release]` in a +# library crate applies only when this crate is the workspace root — i.e. to +# this repo's own tests, examples and benches — and is invisible to downstream +# consumers, who keep their own profile. +[profile.release] +lto = "fat" +codegen-units = 1 + +[profile.bench] +inherits = "release" + [dependencies] rayon = "1" tempfile = "3" diff --git a/src/ext_mem.rs b/src/ext_mem.rs index 621a3ab..1180273 100644 --- a/src/ext_mem.rs +++ b/src/ext_mem.rs @@ -388,8 +388,8 @@ where /// bytes — ~770 MB on the human genome, vs the ~50 GB the equivalent /// `Vec` would take). Phase 1's per-subarray fill is then driven /// by popcount-walking the bitmap; the predicate is **never invoked -/// again** after the initial build. See [`FilteredSource`] for the -/// memory accounting and the inner loop. +/// again** after the initial build. See the crate-internal +/// `FilteredSource` for the memory accounting and the inner loop. /// /// Use this entry when the caller already has the text in RAM and /// the kept positions are described by a cheap per-position diff --git a/src/sample_sort.rs b/src/sample_sort.rs index daa9264..882bc27 100644 --- a/src/sample_sort.rs +++ b/src/sample_sort.rs @@ -422,6 +422,119 @@ mod tests { assert_eq!(got, want, "mismatch on text {text:?}"); } + /// Run the production kernel and return **both** the suffix array and + /// the LCP array it computes as a byproduct. + /// + /// The public entry points discard the LCP array, but it is not an + /// incidental artefact: the next merge level *consumes* it in the + /// three-case decision, so a single wrong LCP entry silently reorders + /// suffixes at the level above. It therefore needs direct coverage. + fn build_sa_and_lcp(text: &[u8], max_ctx: usize) -> (Vec, Vec) { + let n = text.len(); + let mut sa: Vec = (0..n as u32).collect(); + let mut sa_w = vec![0u32; n]; + let mut lcp_arr = vec![0u32; n]; + let mut lcp_w = vec![0u32; n]; + merge_sort( + text, + &PlainText::new(n), + &mut sa, + &mut sa_w, + &mut lcp_arr, + &mut lcp_w, + max_ctx, + LcpDispatch::detect(), + ); + (sa, lcp_arr) + } + + /// Byte-at-a-time LCP of `text[a..]` and `text[b..]`, capped at `max_ctx`. + fn naive_lcp(text: &[u8], a: usize, b: usize, max_ctx: usize) -> usize { + let lim = (text.len() - a).min(text.len() - b).min(max_ctx); + (0..lim).take_while(|&i| text[a + i] == text[b + i]).count() + } + + /// Assert the LCP-array postcondition stated on [`merge_sort`]: + /// `lcp[0] == 0` and `lcp[i] == lcp(text[sa[i-1]..], text[sa[i]..])`. + fn assert_lcp_valid(text: &[u8], max_ctx: usize) { + let (sa, lcp) = build_sa_and_lcp(text, max_ctx); + if sa.is_empty() { + return; + } + assert_eq!(lcp[0], 0, "lcp[0] must be 0 (text {text:?})"); + for i in 1..sa.len() { + let want = naive_lcp(text, sa[i - 1] as usize, sa[i] as usize, max_ctx); + assert_eq!( + lcp[i] as usize, + want, + "lcp[{i}] wrong for sa[{}]={} vs sa[{i}]={} (text {text:?})", + i - 1, + sa[i - 1], + sa[i], + ); + } + } + + #[test] + fn lcp_array_matches_naive_on_fixtures() { + for text in [ + b"banana".as_slice(), + b"mississippi", + b"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + b"abababababababababababababab", + b"a", + b"", + ] { + assert_lcp_valid(text, usize::MAX); + } + } + + #[test] + fn lcp_array_matches_naive_on_random() { + use rand::{RngExt, SeedableRng}; + let mut rng = rand::rngs::StdRng::seed_from_u64(0x1CB0); + for &sigma in &[2u8, 4, 6, 255] { + for &n in &[2usize, 3, 7, 16, 17, 63, 64, 65, 200, 1000, 5000] { + let text: Vec = (0..n).map(|_| rng.random_range(0..sigma)).collect(); + assert_lcp_valid(&text, usize::MAX); + } + } + } + + /// Long runs of one symbol are the worst case for the LCP invariant: + /// adjacent suffixes share almost everything, so every `lcp[i]` is + /// large and an off-by-one is easy to miss. + #[test] + fn lcp_array_on_long_runs_and_periodic_text() { + assert_lcp_valid(&vec![7u8; 2000], usize::MAX); + let periodic: Vec = (0..2000).map(|i| (i % 3) as u8).collect(); + assert_lcp_valid(&periodic, usize::MAX); + // A run embedded in noise, the shape a poly-N genome block has. + let mut mixed: Vec = (0..500).map(|i| (i % 4) as u8).collect(); + mixed.extend(std::iter::repeat_n(4u8, 1500)); + mixed.extend((0..500).map(|i| (i % 4) as u8)); + assert_lcp_valid(&mixed, usize::MAX); + } + + #[test] + fn lcp_array_respects_max_context() { + use rand::{RngExt, SeedableRng}; + let mut rng = rand::rngs::StdRng::seed_from_u64(0xC7A); + for &max_ctx in &[1usize, 2, 4, 16] { + for &n in &[64usize, 500] { + let text: Vec = (0..n).map(|_| rng.random_range(0..3u8)).collect(); + let (sa, lcp) = build_sa_and_lcp(&text, max_ctx); + for i in 1..sa.len() { + let want = naive_lcp(&text, sa[i - 1] as usize, sa[i] as usize, max_ctx); + assert_eq!( + lcp[i] as usize, want, + "lcp[{i}] wrong with max_ctx={max_ctx}" + ); + } + } + } + } + #[test] fn empty_text() { let sa: Vec = build_in_memory::(&[]);