From 164e879585907398828fd0a9be3de7f1b1b32c92 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 20 Aug 2026 18:58:45 +0200 Subject: [PATCH] feat: introduce obikpartition crate for kmer partition layer lookup Introduces the `obikpartition` crate containing the `KmerPartition` struct to manage open layers in sequential order. The `open` constructor eagerly initializes layers under a specified directory, while the `find` method returns the index of the first layer containing a given k-mer. Dependencies are strictly scoped to `obilayeredmap` and `obikseq`, with unit tests validating layer ingestion and single-kmer lookup behavior. Batch lookups and further migration are deferred to a subsequent phase. --- .../implementation/partition_layer_cache.md | 69 +++++++++++++- src/Cargo.lock | 11 +++ src/Cargo.toml | 2 +- src/obikpartition/Cargo.toml | 14 +++ src/obikpartition/src/lib.rs | 69 ++++++++++++++ src/obikpartition/src/tests.rs | 90 +++++++++++++++++++ 6 files changed, 250 insertions(+), 5 deletions(-) create mode 100644 src/obikpartition/Cargo.toml create mode 100644 src/obikpartition/src/lib.rs create mode 100644 src/obikpartition/src/tests.rs diff --git a/DevDocMD/implementation/partition_layer_cache.md b/DevDocMD/implementation/partition_layer_cache.md index b97bcd4b..e3dea710 100644 --- a/DevDocMD/implementation/partition_layer_cache.md +++ b/DevDocMD/implementation/partition_layer_cache.md @@ -2,9 +2,11 @@ Status (2026-08-20, latest pass): (1) done — `obilayeredmap::Layer` exists, `Mat` is gone. (1b) done — `Layer::Empty`, the first non-ready -state, added (panics on every read method). (2) — `KmerPartition` + a -multi-partition cache — **not started**, precisely specified below after -a real mix-up: an earlier +state, added (panics on every read method). (2a) done — the +`obikpartition` crate and `KmerPartition` itself exist (`open`/`n_layers`/ +`layer`/`layers`/`find`). (2b) — migrating `PartitionCache`/`QueryLayer` +onto it — **not started**, deliberately deferred. Earlier mix-up, for +context: an earlier version of this doc used the name `KmerPartition` (singular) for what was actually the *collection* type (later renamed `KmerPartitions`, later merged into `KmerIndex` — see "Major restructuring" below), and never @@ -252,7 +254,66 @@ Full workspace suite green (`cargo check --workspace --all-targets` then Still deferred, per explicit instruction: `build_mphf()`/ `build_unitigs()`/`build_evidence()` to progress `Empty` further, and (2) -— `KmerPartition` itself — unchanged from above. +— `KmerPartition` itself — unchanged from above (see "(2a) done" below, +added next). + +## (2a) done (2026-08-20): `obikpartition` crate + `KmerPartition` + +Built exactly the shape "Definitions" (top of file) specifies, nothing +more — deliberately scoped down from the full "Direction agreed" plan +below: only steps 1–2 (`open`/`n_layers`/`layer`/`layers`/`find`), not 3–4 +(migrating `PartitionCache`/`QueryLayer` onto it), per explicit +instruction to implement `KmerPartition` first and decide the wiring +("comment on branche tout ça dans la construction") separately, later. + +```rust +pub struct KmerPartition { + layers: Vec, +} + +impl KmerPartition { + pub fn open(index_dir: &Path, mode: &IndexMode, n_layers: usize, with_counts: bool) -> OLMResult; + pub fn n_layers(&self) -> usize; + pub fn layer(&self, i: usize) -> &Layer; + pub fn layers(&self) -> &[Layer]; + pub fn find(&self, kmer: CanonicalKmer) -> Option; +} +``` + +`open` takes `index_dir`/`mode`/`n_layers`/`with_counts` as plain +arguments — no reach-back into `KmerIndex` (would need `obikpartition → +obikindex`, the wrong direction) — and builds each layer's path via +`obilayeredmap::layer_dir(index_dir, l)`, the same shared naming +primitive `KmerIndex::layer_dir` itself delegates to, not a second copy of +the `layer_N` convention. `find` mirrors `PartitionCache::find`'s +semantics (first layer that carries the kmer wins) but doesn't yet cover +`find_presence_batch`/`find_presence_batch_fast` — those exist only to +serve `PartitionCache`, so they're part of the (2b) migration, not this +step; building them now against the current sibling-specific tuple shape +`(CanonicalKmer, usize, u8, u8)` would either bake phylo vocabulary +(`family_idx`, `base`) into `obikpartition` or require deciding a generic +payload shape — a real design fork, deferred to when (2b) is actually +tackled rather than guessed at here. + +Crate deps: `obikseq`, `obilayeredmap` only (dev-deps add `obiskio`, +`obicompactvec`, `tempfile` for tests) — matches the "Definitions" +constraint (`obikpartition` depends on `obilayeredmap` and below, never +`obikindex`/`obikpartitionner`/`obikphylo`). Registered as a new workspace +member (`src/Cargo.toml`). 3 new tests (`open_reads_every_layer_in_order`, +`find_reports_the_first_layer_that_carries_the_kmer`, +`find_returns_none_for_an_absent_kmer`). Full workspace suite green +(`cargo check --workspace --all-targets` then `cargo test --workspace`) +after. + +Still not done: (2b) — migrating `obikphylo::siblings::cache:: +PartitionCache` (currently `Vec>`) and +`obikindex::query_layer::QueryLayer` (currently uncached, bypasses `Layer` +entirely) onto `KmerPartition`/`Vec`; deciding whether that +collection lives in `obikpartition` or `obikindex`; deciding the +batch-lookup surface's exact shape (generic payload vs. as-is sibling +tuple moved in wholesale); `scan_layer_families`'s still-independent +`PartitionMeta::load` (see "Remaining instance…" below) — all explicitly +deferred to whenever wiring is tackled next. ## The problem diff --git a/src/Cargo.lock b/src/Cargo.lock index e7ba19b5..a8493d94 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -1655,6 +1655,17 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "obikpartition" +version = "0.1.0" +dependencies = [ + "obicompactvec", + "obikseq", + "obilayeredmap", + "obiskio", + "tempfile", +] + [[package]] name = "obikpartitionner" version = "0.1.0" diff --git a/src/Cargo.toml b/src/Cargo.toml index f34f6918..3ead8c89 100644 --- a/src/Cargo.toml +++ b/src/Cargo.toml @@ -1,5 +1,5 @@ [workspace] resolver = "3" -members = ["obikseq", "obiread", "obiskbuilder", "obifastwrite", "obikmer","obikrope","obipipeline", "obikpartitionner","obiskio","obidebruinj","obilayeredmap", "obicompactvec", "obisys", "obikindex", "obitaxonomy", "obikentropy", "obikphylo"] +members = ["obikseq", "obiread", "obiskbuilder", "obifastwrite", "obikmer","obikrope","obipipeline", "obikpartitionner","obiskio","obidebruinj","obilayeredmap", "obicompactvec", "obisys", "obikindex", "obitaxonomy", "obikentropy", "obikphylo", "obikpartition"] [profile.release] debug = 1 diff --git a/src/obikpartition/Cargo.toml b/src/obikpartition/Cargo.toml new file mode 100644 index 00000000..2ffbe8eb --- /dev/null +++ b/src/obikpartition/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "obikpartition" +version = "0.1.0" +edition = "2024" + +[dependencies] +obikseq = { path = "../obikseq" } +obilayeredmap = { path = "../obilayeredmap" } + +[dev-dependencies] +tempfile = "3" +obikseq = { path = "../obikseq", features = ["test-utils"] } +obiskio = { path = "../obiskio" } +obicompactvec = { path = "../obicompactvec" } diff --git a/src/obikpartition/src/lib.rs b/src/obikpartition/src/lib.rs new file mode 100644 index 00000000..5c10b25d --- /dev/null +++ b/src/obikpartition/src/lib.rs @@ -0,0 +1,69 @@ +//! The **Partition** tier of the `Index { Partition { Layer } }` model — +//! see `DevDocMD/implementation/partition_layer_cache.md`'s "Definitions" +//! section for the authoritative naming/scope discussion this crate +//! implements. `obilayeredmap` already holds the **Layer** tier as its own +//! crate rather than living inside `obikindex`; this crate holds the tier +//! above it the same way. +//! +//! [`KmerPartition`] represents one partition's already-open layers — a +//! read cache, opened once and held for the run, not rebuilt per lookup. +//! It does no path computation of its own beyond the shared +//! `obilayeredmap::layer_dir` naming primitive: which partition, which +//! `index_dir`, how many layers, and `IndexMode` are `KmerIndex`'s +//! (`obikindex`) job to resolve and hand in as plain arguments — this +//! crate depends only on `obilayeredmap` and below, never on `obikindex`, +//! so it cannot reach back for them itself. +//! +//! Not yet wired into any caller: `obikphylo::siblings::cache:: +//! PartitionCache` (`Vec>`) and +//! `obikindex::query_layer::QueryLayer` (uncached, bypasses `Layer` +//! entirely) both still reinvent a fragment of this. Migrating them is a +//! separate, deferred step. + +use std::path::Path; + +use obikseq::CanonicalKmer; +use obilayeredmap::{layer_dir, IndexMode, Layer, OLMResult}; + +/// One partition's open layers, in layer order (layer 0 first). +pub struct KmerPartition { + layers: Vec, +} + +impl KmerPartition { + /// Open every layer under `index_dir` (`index_dir/layer_0`, + /// `index_dir/layer_1`, ... up to `n_layers`), eagerly — not lazily on + /// first access, so the caller pays the mmap cost once, up front, + /// rather than at an unpredictable point during later lookups. + pub fn open(index_dir: &Path, mode: &IndexMode, n_layers: usize, with_counts: bool) -> OLMResult { + let layers = (0..n_layers) + .map(|l| Layer::open(&layer_dir(index_dir, l), mode, with_counts)) + .collect::>>()?; + Ok(Self { layers }) + } + + pub fn n_layers(&self) -> usize { + self.layers.len() + } + + pub fn layer(&self, i: usize) -> &Layer { + &self.layers[i] + } + + pub fn layers(&self) -> &[Layer] { + &self.layers + } + + /// Existence lookup of `kmer` across this partition's layers: tries + /// each in turn, stopping at the first hit and reporting which layer it + /// was — `find_slot`, not a data read, for a plain existence check. + pub fn find(&self, kmer: CanonicalKmer) -> Option { + self.layers + .iter() + .enumerate() + .find_map(|(li, layer)| layer.find_slot(kmer).map(|_| li)) + } +} + +#[cfg(test)] +mod tests; diff --git a/src/obikpartition/src/tests.rs b/src/obikpartition/src/tests.rs new file mode 100644 index 00000000..93027916 --- /dev/null +++ b/src/obikpartition/src/tests.rs @@ -0,0 +1,90 @@ +use super::*; +use obikseq::{set_k, Unitig}; +use obiskio::{UnitigFileWriter, DEFAULT_BLOCK_BITS}; +use tempfile::tempdir; + +fn write_layer(root: &Path, l: usize, seqs: &[&[u8]], n_genomes: usize, mode: &IndexMode) { + let dir = layer_dir(root, l); + std::fs::create_dir_all(&dir).unwrap(); + let mut w = UnitigFileWriter::create(&dir.join("unitigs.bin")).unwrap(); + for s in seqs { + w.write(&Unitig::from_ascii(s)).unwrap(); + } + w.close().unwrap(); + obilayeredmap::TypedLayer::::build_presence( + &dir, + DEFAULT_BLOCK_BITS, + mode, + n_genomes, + |_kmer, _g| true, + ) + .unwrap(); +} + +#[test] +fn open_reads_every_layer_in_order() { + // k=4, process-wide in test builds — see obilayeredmap's own tests for + // why a fixed, unshared k matters here. + set_k(4); + let dir = tempdir().unwrap(); + let mode = IndexMode::Exact; + write_layer(dir.path(), 0, &[b"AAATCTA"], 1, &mode); + write_layer(dir.path(), 1, &[b"CTTCGCC"], 1, &mode); + + let partition = KmerPartition::open(dir.path(), &mode, 2, false).unwrap(); + + assert_eq!(partition.n_layers(), 2); + assert_eq!(partition.layers().len(), 2); +} + +#[test] +fn find_reports_the_first_layer_that_carries_the_kmer() { + set_k(4); + let dir = tempdir().unwrap(); + let mode = IndexMode::Exact; + write_layer(dir.path(), 0, &[b"AAATCTA"], 1, &mode); + write_layer(dir.path(), 1, &[b"CTTCGCC"], 1, &mode); + + let partition = KmerPartition::open(dir.path(), &mode, 2, false).unwrap(); + + let in_layer_0 = obiskio::CanonicalKmerIter::new(&layer_dir(dir.path(), 0).join("unitigs.bin")) + .unwrap() + .next() + .unwrap(); + let in_layer_1 = obiskio::CanonicalKmerIter::new(&layer_dir(dir.path(), 1).join("unitigs.bin")) + .unwrap() + .next() + .unwrap(); + + assert_eq!(partition.find(in_layer_0), Some(0)); + assert_eq!(partition.find(in_layer_1), Some(1)); +} + +#[test] +fn find_returns_none_for_an_absent_kmer() { + set_k(4); + let dir = tempdir().unwrap(); + let mode = IndexMode::Exact; + write_layer(dir.path(), 0, &[b"AAATCTA"], 1, &mode); + + let partition = KmerPartition::open(dir.path(), &mode, 1, false).unwrap(); + + let absent = obiskio::CanonicalKmerIter::new(&layer_dir(dir.path(), 0).join("unitigs.bin")) + .unwrap() + .next() + .unwrap(); + // Not actually absent from the MPHF domain in a general sense, but a + // kmer that was never written to this partition's only layer is what + // "absent" means here — build a second, disjoint layer purely to + // source a genuinely foreign kmer instead of relying on ptr_hash's + // undefined behaviour for out-of-domain queries. + let other_dir = tempdir().unwrap(); + write_layer(other_dir.path(), 0, &[b"GGGGTAC"], 1, &mode); + let foreign = obiskio::CanonicalKmerIter::new(&layer_dir(other_dir.path(), 0).join("unitigs.bin")) + .unwrap() + .next() + .unwrap(); + + assert_eq!(partition.find(absent), Some(0)); + assert_eq!(partition.find(foreign), None); +}