From c4b69e1af5e441bfc3daeb57c44498687d956c7f Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 20 Aug 2026 14:43:10 +0200 Subject: [PATCH] Introduce lifecycle-aware Layer::Empty variant and update callers The `Layer` enum is transformed into a lifecycle-aware state machine with an `Empty` variant representing an unconstructed directory. Read and query operations now explicitly panic when invoked on this state, enforcing explicit progression through `create()` before use. Iterator methods are updated to handle the new variant exhaustively, and module visibility constants are adjusted to support the refactored structure. --- .../implementation/partition_layer_cache.md | 364 ++++++++++++------ src/obikphylo/src/siblings/iter.rs | 4 + src/obilayeredmap/src/content_layer.rs | 71 +++- src/obilayeredmap/src/layer.rs | 2 +- 4 files changed, 311 insertions(+), 130 deletions(-) diff --git a/DevDocMD/implementation/partition_layer_cache.md b/DevDocMD/implementation/partition_layer_cache.md index cb5e1558..b97bcd4b 100644 --- a/DevDocMD/implementation/partition_layer_cache.md +++ b/DevDocMD/implementation/partition_layer_cache.md @@ -1,74 +1,120 @@ # Partition and layer caching (discussion) -Status: problem confirmed, design direction agreed (2026-08-20). (1)/(2) -themselves not started; ownership split (below) still open. Preparatory -encapsulation work (partition/layer path and metadata accessors on -`KmerPartition`) landed the same day — see "Preparatory work done" below. +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 +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 +retracted that usage before this section was rewritten. An agent working +from that stale wording built the wrong thing. **If you are about to +implement (2), read "Definitions: `obikpartition` and `KmerPartition`" +below — it is the current, authoritative naming — before touching any +other section of this file, some of which still describe superseded +states of the code and are kept only as dated history.** + +## Definitions: `obikpartition` and `KmerPartition` (not yet created) + +**`obikpartition`** — a new workspace crate, not created yet. Holds the +**Partition** tier of the `Index { Partition { Layer } }` model, the same +way `obilayeredmap` already holds the **Layer** tier as its own crate +rather than living inside `obikindex`. Depends only on `obilayeredmap` +(for `Layer`) and lower (`obikseq`, `obiskio`). Does **not** depend on +`obikindex`, `obikpartitionner`, or `obikphylo`. Dependency direction: +`obikindex → obikpartition → obilayeredmap`; `obikphylo → obikindex` +(and/or `obikpartition` directly if it ends up needing it without going +through `KmerIndex`). + +**`KmerPartition`** (singular) — the one type this crate exists for. +Represents **one partition's already-open layers** — a read cache, built +once per partition and held for the run, not rebuilt per lookup. Shape: + +```rust +pub struct KmerPartition { + layers: Vec, +} +``` + +Nothing else. In particular: +- **No path computation.** `KmerPartition::open` takes an already-resolved + `index_dir: &Path` (plus `mode: &IndexMode`, `n_layers: usize`, + `with_counts: bool` — whatever it needs, as plain arguments), the same + discipline `obikpartitionner::PartitionRouter::open` already follows. + Computing `index_dir`/`layer_dir` from a partition number is + `KmerIndex`'s job (`obikindex`, which owns that already — see "Major + restructuring" below); `KmerPartition` never reaches back into + `KmerIndex` to get it (would require `obikpartition → obikindex`, the + wrong direction). +- **No routing/write state.** Writing raw superkmers, `dereplicate`, + `count_kmer` stay in `obikpartitionner::PartitionRouter` — a completely + different crate, a completely different phase (pre-layer, whereas + `KmerPartition` only makes sense once layers exist). +- **No multi-partition collection baked in.** `KmerPartition` is *one* + partition. Whatever ends up caching several of them (replacing + `obikphylo::siblings::cache::PartitionCache`'s `Vec>` and + `obikindex::query_layer`'s per-call reopen) holds `Vec` — + that collection can live in `obikpartition` too, or in `obikindex` + alongside `KmerIndex`; not yet decided, secondary to getting + `KmerPartition` itself right first. + +**Do not confuse with `KmerPartitions`** (plural — note the `s`): that +type is **gone**. It used to be `obikpartitionner`'s (nee `obikpartition`, +briefly — see the crate-rename history below, itself a separate rename +from this one) do-everything struct — routing, dereplication, *and* path +lookups all in one. It was deleted on 2026-08-20; its read-side (paths, +`n_layers`, `partition_meta`) was absorbed into `KmerIndex`, its +write-side became `PartitionRouter`. `KmerPartition` (this section, +singular, no final `s`) is a brand-new type with a different job, in a +crate that doesn't exist yet — not a revival, not a renaming, of +`KmerPartitions`. ## Type-to-concept mapping: Index / Partition / Layer The conceptual nesting `Index { Partition { Layer { MPHF, Evidence, Matrix -} } } }` does **not** have one Rust type per level — worth stating -explicitly, since two of the names below are misleading. +} } } }`, current state: -- **Index** = `obikindex::KmerIndex` — `{ root_path, meta: IndexMeta, - partition: KmerPartition }`. The field is named `partition` (singular), - but it holds the *whole* multi-partition structure below — the name - suggests "one partition", the value is all of them. -- **Partition, the collection (not one partition)** = - `obikpartition::KmerPartition`. Despite the singular name, this owns - *every* partition of the index: `root_path`, `n_partitions`, and - per-partition accessors that all take an explicit index `i` +- **Index** = `obikindex::KmerIndex` — `{ root_path, meta: IndexMeta }`. + Also directly exposes the partition-path/metadata accessors (`partition_dir(i)`, `index_dir(i)`, `layer_dir(i, l)`, - `partition_meta(i)`, `n_layers(i)`, `index_mode(i)` — see the - `part_dir`/`layer_dir` and `PartitionMeta`-encapsulation work above). - It never holds one partition's layers open in memory — no `Vec>` - here, only path arithmetic and metadata reads. A more honest name would - be `KmerPartitionSet` or `KmerPartitioner`; renaming is out of scope for - now, just worth knowing it's not "a partition." -- **Partition, one of them** = **no dedicated type exists today**. The - structural equivalent of "one partition, its layers held open" is - `obilayeredmap::LayeredMap` — `{ root, meta: PartitionMeta, layers: - Vec> }` — but `LayeredMap` itself doesn't know it's playing that - role: it operates purely on whatever `root` path it's given and has no - notion of `KmerPartition`, `n_partitions`, or a partition index `i` (see - "Layering: who owns what" below — established independently while - fixing `open_data`/`layer_dir`). Every caller that wants "one partition, - opened" builds the path itself - (`kmer_partition.index_dir(i)`/`.layer_dir(i, l)`) and either passes it - to `LayeredMap::open` or — as `obikphylo::siblings::cache` does — - bypasses `LayeredMap` entirely and opens each `Layer` directly. -- **Layer** = `obilayeredmap::Layer` — `{ mphf: MphfLayer, data: D }`. + `partition_meta(i)`, `n_layers(i)`, `partition_mode(i)`, + `n_partitions()`) since `KmerPartitions` merged into it (see "Major + restructuring" below) — `KmerIndex` today *is* "index + collection of + partitions' paths & metadata," just without a `Vec` of open layers. +- **Partition, the collection** = no dedicated type today; the closest + thing is `KmerIndex` itself (previous bullet). Once `KmerPartition` + (singular, see Definitions above) exists, a `Vec` + somewhere would be this — still open, see "Direction agreed" below. +- **Partition, one of them** = `obikpartition::KmerPartition` — **to be + built**, see Definitions above. Nothing plays this role today; + `obikphylo::siblings::cache::PartitionCache` and + `obikindex::query_layer::QueryLayer` each independently reinvent a + fragment of it. +- **Layer** = `obilayeredmap::Layer` (format-erased: `Count`/`Presence`, + each wrapping a `TypedLayer`) — see "(1) done" below for how this + came to be; `TypedLayer` (`{ mphf: MphfLayer, data: D }`, monomorphic) + is the lower-level, `D`-fixed building block `Layer` is built on, not + what other crates should reach for directly. - **MPHF** = `MphfLayer.mphf: MemCase` — kmer → slot. - **Evidence** = `MphfLayer.ev: LayerEvidence` (`Exact`/`Approx`/ `Hybrid` — `evidence.bin`/`fingerprint.bin`; see `EvidenceKind`). - - **Matrix** = `Layer.data: D` — `PersistentBitMatrix` / - `PersistentCompactIntMatrix` / `PersistentSparseBitMatrix` / `()`. + - **Matrix** = `TypedLayer.data: D` — `PersistentBitMatrix` / + `PersistentCompactIntMatrix`. -Today's actual nesting, in Rust terms: +Target nesting once `KmerPartition` exists: ``` -KmerIndex - └─ partition: KmerPartition (all N partitions — paths + metadata only) - └─ (opened on demand, per i) LayeredMap ← "one partition", unnamed as such - └─ layers: Vec> - └─ Layer { mphf: MphfLayer, data: D } +KmerIndex (obikindex) + └─ (opened on demand, per i) KmerPartition (obikpartition — not yet built) + └─ layers: Vec (obilayeredmap) + └─ Layer::Count/Presence(TypedLayer) + └─ TypedLayer { mphf: MphfLayer, data: D } ├─ mphf.mphf → MPHF ├─ mphf.ev → Evidence └─ data → Matrix ``` -`obikphylo::siblings::cache::PartitionCache` — the thing (2) is meant to -generalise — skips the middle of this nesting entirely: it doesn't build a -`Vec>`, it builds `mats: Vec>` directly — outer -index = partition `i` (via `KmerPartition`), inner index = layer `l` (via -`Layer`/`Mat`) — reconstructing "one partition, its layers open" as a -bare nested `Vec` because no owning type for that concept exists to reuse. -That gap is exactly what (2) needs to fill, and (1) is exactly what would -let a per-partition type hold layers of mixed `D` (today `LayeredMap` -can't, being monomorphic — see "The gap in `obilayeredmap`'s existing -cache" below). - ## Major restructuring (2026-08-20): `KmerPartitions` merged into `KmerIndex` Prompted by a direct question: why keep `KmerIndex`/`KmerPartitions` split @@ -163,6 +209,51 @@ still-separate `QueryLayer` (which still independently bundles MPHF+matrix, 2-way not using `Layer` at all). Both remaining consumers now sit one `Layer::open` call away from unifying onto (2) once it exists. +## (1b) done (2026-08-20): `Layer::Empty` — the first non-ready-to-read state + +First step toward `Layer` representing a layer's whole life, not just the +open-for-reading end of it (see "Definitions" above: `KmerPartition` will +hold `Vec` regardless of each layer's state, states in between +included). Added one variant: + +```rust +pub enum Layer { + Empty { dir: PathBuf }, + Count(TypedLayer), + Presence(TypedLayer), +} +``` + +`Layer::create(dir)` makes the directory and returns `Empty { dir }` — +nothing else; no MPHF/unitigs/evidence construction yet (that's the +deferred next step: `build_mphf()`/`build_unitigs()`/`build_evidence()` +methods to progress `Empty` → eventually `Count`/`Presence`). `Empty` +carries path accessors so builder code has one place to get +`mphf_path()`/`unitigs_path()`/`evidence_path()`/`fingerprint_path()`/ +`counts_dir()`/`presence_dir()` from, instead of redeclaring the +`mphf.bin`/`unitigs.bin`/… filenames at each write site — reusing the +constants `layer.rs`/`mphf_layer.rs` already own (`COUNTS_DIR`/ +`PRESENCE_DIR` widened from private to `pub(crate)`, file-name constants +already were). + +Every read method (`content`/`evidence_kind`/`n`/`find_slot`/ +`index_batch`/`n_cols`/`fill_sub_matrix_carries`) panics on `Empty` with a +one-line message naming the method — confirmed as the right behaviour: +calling any of them on an `Empty` layer means the caller assumed a layer +was ready when it wasn't, an implementation error to surface loudly, not +a case to design around (`Option`/`Result` would let it silently +propagate instead of failing at the actual mistake). Same panic added to +`obikphylo::siblings::iter.rs`'s `impl SiblingLayerExt for +obilayeredmap::Layer` (4 methods), the one other place that exhaustively +matched `Layer`'s variants. + +Full workspace suite green (`cargo check --workspace --all-targets` then +`cargo test --workspace`, exit code 0) after. + +Still deferred, per explicit instruction: `build_mphf()`/ +`build_unitigs()`/`build_evidence()` to progress `Empty` further, and (2) +— `KmerPartition` itself — unchanged from above. + ## The problem Reading a layer's data (MPHF + matrix) is not free: `MphfLayer::open` mmaps @@ -180,34 +271,47 @@ across cores without reducing it. `PartitionCache::build` opens every partition's every layer once, up front, in parallel, and keeps the handles alive for the run. -## Three independent implementations of the same bundle +## Three independent implementations of the same bundle (historical — (1) fixed this) + +**As of 2026-08-20 this table describes the pre-(1) state.** `Mat` no +longer exists (deleted when `obilayeredmap::Layer` replaced it — see "(1) +done" above); `Layer` in the table below is what's now called +`TypedLayer`. `QueryLayer` is unaffected and still stands as described — +still uncached, still not using `Layer` at all — which is exactly what (2) +needs to fix. Kept for the original motivation, not as current fact: Searching the codebase for "who bundles MPHF + matrix, with per-layer format -auto-detection" turns up three unrelated implementations: +auto-detection" turned up three unrelated implementations: | | lives in | scope | cached? | |---|---|---|---| -| `Layer` | `obilayeredmap` | one layer, `D` fixed at compile time | held alive by whoever owns the `Layer`, no policy of its own | -| `Mat` | `obikphylo::siblings::cache` | one layer, format resolved per instance from an enum of 3 `Layer` variants | yes, via `PartitionCache` | -| `QueryLayer` | `obikpartition::query_layer` | one layer, `(MphfLayer, PersistentBitMatrix\|PersistentCompactIntMatrix)` pair, bypasses `Layer` entirely | **no** — opened fresh inside `query_partition_with` on every call | +| `Layer` (now `TypedLayer`) | `obilayeredmap` | one layer, `D` fixed at compile time | held alive by whoever owns the `Layer`, no policy of its own | +| `Mat` (now deleted; superseded by `obilayeredmap::Layer`) | `obikphylo::siblings::cache` | one layer, format resolved per instance from an enum of 3 `Layer` variants | yes, via `PartitionCache` | +| `QueryLayer` (unchanged, still current) | `obikindex::query_layer` (moved crates since this was written — see "Major restructuring") | one layer, `(MphfLayer, PersistentBitMatrix\|PersistentCompactIntMatrix)` pair, bypasses `TypedLayer`/`Layer` entirely | **no** — opened fresh inside `query_partition_with` on every call | `query_partition_with` is `obikmer query`'s normal query path — the one -most exposed to repeated cross-partition lookups — and it is the one with -no cache at all. `obikphylo` built a cache first only because sibling-annex -construction hits the cost hardest, not because the need is sibling-specific. +most exposed to repeated cross-partition lookups — and it is still the one +with no cache at all. `obikphylo` built a cache first only because +sibling-annex construction hits the cost hardest, not because the need is +sibling-specific. -## The gap in `obilayeredmap`'s existing cache +## The gap in `obilayeredmap`'s existing cache (historical — (1) fixed this) `obilayeredmap::LayeredMap` already caches correctly at the granularity -of one partition: `open(root)` opens every layer once, keeps `Vec>` -alive for the `LayeredMap`'s lifetime. But it is monomorphic — every layer -in the `Vec` must share the same concrete `D`. In practice this is false: -layers in the same partition are packed independently over time (`pack ---sparse` converts one layer's presence matrix at a time), so a partition -can genuinely mix `PersistentBitMatrix`-backed and -`PersistentSparseBitMatrix`-backed presence layers. `LayeredMap` cannot -represent that mix — `Mat`'s 3-way enum exists specifically to work around -this gap, one crate away from where the gap actually is. +of one partition: `open(root)` opens every layer once, keeps +`Vec>` alive for the `LayeredMap`'s lifetime. But it is +monomorphic — every layer in the `Vec` must share the same concrete `D`. +In practice this was false: layers in the same partition are packed +independently over time (`pack --sparse` converts one layer's presence +matrix at a time). This motivated (1) — `obilayeredmap::Layer`, done — but +note the specific `PersistentSparseBitMatrix`-mixing scenario described +here turned out to be moot: `PersistentBitMatrix` itself absorbed sparse +storage as a 4th internal variant before (1) was built (see "One bug found +… one earlier claim retracted" below), so the only heterogeneity `Layer` +actually needs to represent is `Count` vs. `Presence`, not dense-vs-sparse +presence. `LayeredMap` itself is unaffected by any of this — it's still +monomorphic, still not used by `Layer`/`KmerPartition` (which bypass it +entirely, opening each `TypedLayer` directly, the same way `Mat` did). ## Resource cost: mmap does not hold a file descriptor @@ -249,56 +353,65 @@ independently of any caching decision: an unpacked `Columnar` matrix opens one mmap **per genome column**, multiplying the mapping count a cache would have to hold by `n_genomes`. -## Layering: who owns what +## Layering: who owns what (superseded — see Definitions above) -Established in discussion, not yet coded: - -- `obilayeredmap` operates within a single partition's index root; it has - no notion of "partition" (confirmed independently while fixing - `open_data`/`layer_dir` — see git history around 2026-08-20). One - `LayeredMap` (or its heterogeneous-`D` successor) = one partition. -- Nobody currently owns "the collection of partitions" as reusable state. - `obikpartition::KmerPartition` is the closest candidate — it already - owns `n_partitions()`/`part_dir(i)` — but today it is a pure - config/path resolver, not a state holder: `dereplicate`, `count_kmer`, - `obikindex`'s `distance.rs`/`stats.rs`, and - `obikphylo::PartitionCache::build` each independently write their own - `(0..n_partitions).into_par_iter().map(...)` loop and open what they need - from scratch. +This section used to argue nobody owned "the collection of partitions." +That's resolved: `KmerIndex` (`obikindex`) owns it now, directly (see +"Major restructuring" below). What's still genuinely unowned is *one +partition's open layers* — `KmerPartition`, in the not-yet-created +`obikpartition` — see "Definitions" at the top of this file for the +current, authoritative answer. Left here only so old links/references to +this heading don't 404; don't read this section for current facts. ## Direction agreed, not yet implemented -1. A heterogeneous-format layer type in `obilayeredmap` (name TBD), - generalising `Mat`'s 3-way enum + auto-detection — `obilayeredmap` - already implements `LayerData` for all three concrete matrix types - involved, so it already has the knowledge `Mat` needed and had to - duplicate. -2. A cache spanning multiple partitions, keeping (2)'s layer type open per - partition per layer, for the whole lifetime of a long-running command — - generalising `PartitionCache` minus its sibling-specific parts - (`fast_mode`, `find_presence_batch`) — living in `obikpartition` - (owner of the partition dimension) and built on top of (1). -3. `obikpartition::query_partition_with` — the normal query path, - currently uncached — becomes a consumer of (2), not just - `obikphylo::PartitionCache`. +Only (2) remains — (1) shipped as `obilayeredmap::Layer` (see "(1) done" +above). Concretely, in order: -Open before implementing: exact API shape of (1) and (2), whether `Mat` is -deleted outright or kept as a thin sibling-specific wrapper, and whether (2) -needs an eviction policy or can simply hold every partition open for the -process lifetime (revisit once the VM-mapping-count question above has a -real number behind it for this codebase's scale). +1. Create the `obikpartition` crate (`obikindex → obikpartition → + obilayeredmap`, no other edges — see "Definitions" above for the exact + constraint and why). +2. `KmerPartition { layers: Vec }` — `open`, + `n_layers`, `layer(i)`, `find`, plus whatever batch-lookup surface + `obikphylo::siblings::cache::PartitionCache` currently needs + (`find_presence_batch`/`find_presence_batch_fast`; `fast_mode` is + sibling-specific bookkeeping and should probably stay in `obikphylo`, + wrapping a `KmerPartition`/`Vec` rather than living + inside it — same "generic vs. domain-specific" split `iter_minorants_batch` + already went through for `Layer` in (1)). +3. Migrate `obikphylo::siblings::cache::PartitionCache` to hold + `Vec` instead of `Vec>`. +4. Migrate `obikindex::query_layer::QueryLayer`/`query_partition_with` to + use `KmerPartition` too, closing the "no cache at all" gap on + `obikmer`'s normal query path (see "Three independent implementations," + historical, above). + +Open before implementing: exact API shape of `KmerPartition` (propose, +confirm before coding — non-trivial), and whether the multi-partition +`Vec` needs an eviction policy or can simply hold every +partition open for the process lifetime (revisit once the +VM-mapping-count question above has a real number behind it for this +codebase's scale — still not measured). ## Preparatory work done (2026-08-20) -Groundwork for (1)/(2), landed ahead of the design itself: +Groundwork for (1)/(2), landed ahead of the design itself. **Note**: at +the time this was written, the collection type these bullets describe was +named `KmerPartition` (singular) in this doc; it was renamed +`KmerPartitions` (plural) shortly after, then deleted entirely and merged +into `KmerIndex` (see "Major restructuring" above). The bullets below are +edited to say `KmerPartitions` throughout, to not collide with the +unrelated, brand-new singular `KmerPartition` defined at the top of this +file — the accessors described here live on `KmerIndex` today, not on +any type called `KmerPartition`. -- `KmerPartition` (`obikpartition`) gained `partition_dir`/`index_dir`/ - `layer_dir` as the single source of truth for a partition's on-disk - layout, replacing per-module duplicated `const INDEX_SUBDIR: &str = - "index"` (7 copies) and ad hoc path joins — including one found - duplicated *inside `KmerPartition` itself* (`ensure_writer` rebuilt - `part_dir`'s own logic by hand). -- `KmerPartition` gained `partition_meta`/`n_layers`/`index_mode`, wrapping +- `KmerPartitions` (`obikpartitionner`, at the time) gained + `partition_dir`/`index_dir`/`layer_dir` as the single source of truth + for a partition's on-disk layout, replacing per-module duplicated + `const INDEX_SUBDIR: &str = "index"` (7 copies) and ad hoc path joins — + including one found duplicated *inside the struct itself* + (`ensure_writer` rebuilt `part_dir`'s own logic by hand). +- Same struct gained `partition_meta`/`n_layers`/`index_mode`, wrapping `obilayeredmap::meta::PartitionMeta::load` (via the existing `common::load_meta`, which also recovers indexes built before `meta.json` existed). Before this, `obikphylo` and `obikindex` imported @@ -392,16 +505,19 @@ storage decisions, now a *third* copy of the same logic to keep in sync) `obikphylo::siblings::family_scan::scan_layer_families` still re-derives `index_dir` from `layer_dir.parent()` and calls `PartitionMeta::load` -itself, purely to get `.mode` for `Mat::open`. Fixing it the way the 21 -other call sites were fixed needs more than a 1:1 swap: `scan_layer_families` -only receives a bare `layer_dir: &Path`, not a `(partition, part, layer)` -triple, and its single upstream source of layer paths, -`sibling_layer_dirs`, returns a flat `Vec` with the partition/layer -indices already discarded. Fixing it properly means either having -`sibling_layer_dirs` return `(PathBuf, IndexMode)` (or `(part, layer)`) -pairs, or threading `&KmerPartition` + indices through instead of paths — -and touching every one of `scan_layer_families`'s 8 callers (`distance.rs`, -`alignment.rs`, `cardinality.rs`, `entropy.rs` ×2, `sankoff_bundle.rs` ×2, -`stats.rs`). Left alone this round; worth doing as part of the same pass -that builds (1)/(2), since those callers are exactly the sibling-annex -consumers (2) is meant to serve. +itself, purely to get `.mode` for `obilayeredmap::Layer::open` (was +`Mat::open`, same gap, survived the `Mat` → `Layer` swap in (1) unchanged). +Fixing it the way the 21 other call sites were fixed needs more than a 1:1 +swap: `scan_layer_families` only receives a bare `layer_dir: &Path`, not a +`(partition, part, layer)` triple, and its single upstream source of layer +paths, `sibling_layer_dirs`, returns a flat `Vec` with the +partition/layer indices already discarded. Fixing it properly means either +having `sibling_layer_dirs` return `(PathBuf, IndexMode)` (or `(part, +layer)`) pairs, or threading `&KmerIndex` + indices through instead of +paths (not `&KmerPartition` — that type doesn't exist yet, and once it +does it still won't know `IndexMode`, which lives on `KmerIndex`/ +`PartitionMeta`) — and touching every one of `scan_layer_families`'s 8 +callers (`distance.rs`, `alignment.rs`, `cardinality.rs`, `entropy.rs` ×2, +`sankoff_bundle.rs` ×2, `stats.rs`). Left alone this round; worth doing as +part of the same pass that builds `KmerPartition`, since those callers are +exactly the sibling-annex consumers it's meant to serve. diff --git a/src/obikphylo/src/siblings/iter.rs b/src/obikphylo/src/siblings/iter.rs index a3a7cc2c..7e69ca61 100644 --- a/src/obikphylo/src/siblings/iter.rs +++ b/src/obikphylo/src/siblings/iter.rs @@ -183,6 +183,7 @@ impl SiblingLayerExt for obilayeredmap::Layer { match self { obilayeredmap::Layer::Count(l) => l.iter_siblings(annex), obilayeredmap::Layer::Presence(l) => l.iter_siblings(annex), + obilayeredmap::Layer::Empty { .. } => panic!("iter_siblings() called on an Empty layer"), } } @@ -190,6 +191,7 @@ impl SiblingLayerExt for obilayeredmap::Layer { match self { obilayeredmap::Layer::Count(l) => l.iter_siblings_batch(annex, batch_size), obilayeredmap::Layer::Presence(l) => l.iter_siblings_batch(annex, batch_size), + obilayeredmap::Layer::Empty { .. } => panic!("iter_siblings_batch() called on an Empty layer"), } } @@ -197,6 +199,7 @@ impl SiblingLayerExt for obilayeredmap::Layer { match self { obilayeredmap::Layer::Count(l) => l.iter_minorants(annex), obilayeredmap::Layer::Presence(l) => l.iter_minorants(annex), + obilayeredmap::Layer::Empty { .. } => panic!("iter_minorants() called on an Empty layer"), } } @@ -204,6 +207,7 @@ impl SiblingLayerExt for obilayeredmap::Layer { match self { obilayeredmap::Layer::Count(l) => l.iter_minorants_batch(annex, batch_size), obilayeredmap::Layer::Presence(l) => l.iter_minorants_batch(annex, batch_size), + obilayeredmap::Layer::Empty { .. } => panic!("iter_minorants_batch() called on an Empty layer"), } } } diff --git a/src/obilayeredmap/src/content_layer.rs b/src/obilayeredmap/src/content_layer.rs index aa8144a1..80eb65a0 100644 --- a/src/obilayeredmap/src/content_layer.rs +++ b/src/obilayeredmap/src/content_layer.rs @@ -12,25 +12,50 @@ //! reimplement locally, minus its one sibling-specific method //! (`iter_minorants_batch`, which stays an extension trait over there — //! `obilayeredmap` has no business knowing about sibling annexes). +//! +//! `Layer` is meant to eventually represent a layer's *whole* life — +//! empty shell, under construction, ready to read — not just the +//! ready-to-read state, so the same type follows a layer from creation +//! through querying instead of construction living as disconnected free +//! functions elsewhere that happen to write into the same directory (see +//! `DevDocMD/implementation/partition_layer_cache.md`). [`Layer::Empty`] +//! is the first step: a directory and nothing else, able to hand out the +//! paths a builder needs, not yet able to build anything itself. -use std::path::Path; +use std::path::{Path, PathBuf}; use obicompactvec::{PersistentBitMatrix, PersistentCompactIntMatrix}; use obikseq::CanonicalKmer; use crate::error::OLMResult; -use crate::layer::{LayerContent, TypedLayer, COUNTS_DIR}; +use crate::layer::{LayerContent, TypedLayer, COUNTS_DIR, PRESENCE_DIR}; use crate::meta::IndexMode; -use crate::mphf_layer::EvidenceKind; +use crate::mphf_layer::{EvidenceKind, EVIDENCE_FILE, FINGERPRINT_FILE, MPHF_FILE, UNITIGS_FILE}; -/// One layer, its content (`Count`/`Presence`) resolved at open time rather -/// than at compile time — see the module docs. +/// One layer, at any point in its life — see the module docs. Only +/// [`Empty`](Layer::Empty) and the two ready-to-read states +/// (`Count`/`Presence`) exist so far; states in between (unitigs written, +/// MPHF built, evidence built, matrix not yet built) are not represented +/// yet. pub enum Layer { + /// Just a directory — nothing built yet. Every method below other than + /// the path accessors panics on this variant: calling them means the + /// caller assumed a layer was ready when it wasn't, an implementation + /// error to surface loudly, not paper over with a default value. + Empty { dir: PathBuf }, Count(TypedLayer), Presence(TypedLayer), } impl Layer { + /// An empty shell at `dir` — creates the directory (if it doesn't + /// already exist) but nothing inside it. The starting state for + /// building a new layer. + pub fn create(dir: &Path) -> std::io::Result { + std::fs::create_dir_all(dir)?; + Ok(Layer::Empty { dir: dir.to_owned() }) + } + /// Open one layer, auto-detecting count vs. presence from what is /// actually on disk (`counts/` present and wanted, else presence) — the /// single source of truth every caller that opens a layer's own matrix @@ -46,10 +71,40 @@ impl Layer { TypedLayer::::open(layer_dir, mode).map(Layer::Presence) } + // ── Paths — only meaningful before anything is built ─────────────── + // + // Once a layer is `Count`/`Presence`, its caller already knows the + // directory (it had to pass it to `open`) — these exist for builder + // code holding an `Empty` layer, so the `mphf.bin`/`unitigs.bin`/… + // naming stays defined once, here, rather than re-declared as + // string literals at every call site that writes into a layer + // directory (the same duplication `layer_dir`/`index_dir` fixed one + // level up — see `DevDocMD/implementation/partition_layer_cache.md`). + + /// This layer's own directory. Panics on `Count`/`Presence` — by that + /// point the caller already has the directory it opened with; asking + /// again here would mean it lost track of its own state. + pub fn dir(&self) -> &Path { + match self { + Layer::Empty { dir } => dir, + _ => panic!("Layer::dir() only available on Empty — caller already has this path"), + } + } + + pub fn mphf_path(&self) -> PathBuf { self.dir().join(MPHF_FILE) } + pub fn unitigs_path(&self) -> PathBuf { self.dir().join(UNITIGS_FILE) } + pub fn evidence_path(&self) -> PathBuf { self.dir().join(EVIDENCE_FILE) } + pub fn fingerprint_path(&self) -> PathBuf { self.dir().join(FINGERPRINT_FILE) } + pub fn counts_dir(&self) -> PathBuf { self.dir().join(COUNTS_DIR) } + pub fn presence_dir(&self) -> PathBuf { self.dir().join(PRESENCE_DIR) } + + // ── Ready-only surface ─────────────────────────────────────────────── + pub fn content(&self) -> LayerContent { match self { Layer::Count(_) => LayerContent::Count, Layer::Presence(_) => LayerContent::Presence, + Layer::Empty { .. } => panic!("Layer::content() called on an Empty layer"), } } @@ -57,6 +112,7 @@ impl Layer { match self { Layer::Count(l) => l.evidence_kind(), Layer::Presence(l) => l.evidence_kind(), + Layer::Empty { .. } => panic!("Layer::evidence_kind() called on an Empty layer"), } } @@ -64,6 +120,7 @@ impl Layer { match self { Layer::Count(l) => l.n(), Layer::Presence(l) => l.n(), + Layer::Empty { .. } => panic!("Layer::n() called on an Empty layer"), } } @@ -71,6 +128,7 @@ impl Layer { match self { Layer::Count(l) => l.find_slot(kmer), Layer::Presence(l) => l.find_slot(kmer), + Layer::Empty { .. } => panic!("Layer::find_slot() called on an Empty layer"), } } @@ -81,6 +139,7 @@ impl Layer { match self { Layer::Count(l) => l.index_batch(kmers), Layer::Presence(l) => l.index_batch(kmers), + Layer::Empty { .. } => panic!("Layer::index_batch() called on an Empty layer"), } } @@ -88,6 +147,7 @@ impl Layer { match self { Layer::Count(l) => l.n_cols(), Layer::Presence(l) => l.n_cols(), + Layer::Empty { .. } => panic!("Layer::n_cols() called on an Empty layer"), } } @@ -108,6 +168,7 @@ impl Layer { o.extend(c.iter().map(|&v| v != 0)); } } + Layer::Empty { .. } => panic!("Layer::fill_sub_matrix_carries() called on an Empty layer"), } } } diff --git a/src/obilayeredmap/src/layer.rs b/src/obilayeredmap/src/layer.rs index 142b3a1f..3c5428b6 100644 --- a/src/obilayeredmap/src/layer.rs +++ b/src/obilayeredmap/src/layer.rs @@ -17,7 +17,7 @@ use crate::mphf_layer::MphfLayer; pub(crate) use crate::mphf_layer::UNITIGS_FILE; pub(crate) const COUNTS_DIR: &str = "counts"; -const PRESENCE_DIR: &str = "presence"; +pub(crate) const PRESENCE_DIR: &str = "presence"; // ── Trait ─────────────────────────────────────────────────────────────────────