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.
This commit is contained in:
Eric Coissac
2026-08-20 20:41:48 +02:00
parent 1c54e60c9a
commit c4b69e1af5
4 changed files with 311 additions and 130 deletions
+240 -124
View File
@@ -1,74 +1,120 @@
# Partition and layer caching (discussion) # Partition and layer caching (discussion)
Status: problem confirmed, design direction agreed (2026-08-20). (1)/(2) Status (2026-08-20, latest pass): (1) done — `obilayeredmap::Layer`
themselves not started; ownership split (below) still open. Preparatory exists, `Mat` is gone. (1b) done — `Layer::Empty`, the first non-ready
encapsulation work (partition/layer path and metadata accessors on state, added (panics on every read method). (2) — `KmerPartition` + a
`KmerPartition`) landed the same day — see "Preparatory work done" below. 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<obilayeredmap::Layer>,
}
```
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<Vec<Layer>>` and
`obikindex::query_layer`'s per-call reopen) holds `Vec<KmerPartition>`
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 ## Type-to-concept mapping: Index / Partition / Layer
The conceptual nesting `Index { Partition { Layer { MPHF, Evidence, Matrix The conceptual nesting `Index { Partition { Layer { MPHF, Evidence, Matrix
} } } }` does **not** have one Rust type per level — worth stating } } } }`, current state:
explicitly, since two of the names below are misleading.
- **Index** = `obikindex::KmerIndex` — `{ root_path, meta: IndexMeta, - **Index** = `obikindex::KmerIndex``{ root_path, meta: IndexMeta }`.
partition: KmerPartition }`. The field is named `partition` (singular), Also directly exposes the partition-path/metadata accessors
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`
(`partition_dir(i)`, `index_dir(i)`, `layer_dir(i, l)`, (`partition_dir(i)`, `index_dir(i)`, `layer_dir(i, l)`,
`partition_meta(i)`, `n_layers(i)`, `index_mode(i)` — see the `partition_meta(i)`, `n_layers(i)`, `partition_mode(i)`,
`part_dir`/`layer_dir` and `PartitionMeta`-encapsulation work above). `n_partitions()`) since `KmerPartitions` merged into it (see "Major
It never holds one partition's layers open in memory — no `Vec<Layer<D>>` restructuring" below) — `KmerIndex` today *is* "index + collection of
here, only path arithmetic and metadata reads. A more honest name would partitions' paths & metadata," just without a `Vec` of open layers.
be `KmerPartitionSet` or `KmerPartitioner`; renaming is out of scope for - **Partition, the collection** = no dedicated type today; the closest
now, just worth knowing it's not "a partition." thing is `KmerIndex` itself (previous bullet). Once `KmerPartition`
- **Partition, one of them** = **no dedicated type exists today**. The (singular, see Definitions above) exists, a `Vec<KmerPartition>`
structural equivalent of "one partition, its layers held open" is somewhere would be this — still open, see "Direction agreed" below.
`obilayeredmap::LayeredMap<D>` — `{ root, meta: PartitionMeta, layers: - **Partition, one of them** = `obikpartition::KmerPartition` — **to be
Vec<Layer<D>> }` — but `LayeredMap` itself doesn't know it's playing that built**, see Definitions above. Nothing plays this role today;
role: it operates purely on whatever `root` path it's given and has no `obikphylo::siblings::cache::PartitionCache` and
notion of `KmerPartition`, `n_partitions`, or a partition index `i` (see `obikindex::query_layer::QueryLayer` each independently reinvent a
"Layering: who owns what" below — established independently while fragment of it.
fixing `open_data`/`layer_dir`). Every caller that wants "one partition, - **Layer** = `obilayeredmap::Layer` (format-erased: `Count`/`Presence`,
opened" builds the path itself each wrapping a `TypedLayer<D>`) — see "(1) done" below for how this
(`kmer_partition.index_dir(i)`/`.layer_dir(i, l)`) and either passes it came to be; `TypedLayer<D>` (`{ mphf: MphfLayer, data: D }`, monomorphic)
to `LayeredMap::open` or — as `obikphylo::siblings::cache` does — is the lower-level, `D`-fixed building block `Layer` is built on, not
bypasses `LayeredMap` entirely and opens each `Layer<D>` directly. what other crates should reach for directly.
- **Layer** = `obilayeredmap::Layer<D>` — `{ mphf: MphfLayer, data: D }`.
- **MPHF** = `MphfLayer.mphf: MemCase<MphfEps>` — kmer → slot. - **MPHF** = `MphfLayer.mphf: MemCase<MphfEps>` — kmer → slot.
- **Evidence** = `MphfLayer.ev: LayerEvidence` (`Exact`/`Approx`/ - **Evidence** = `MphfLayer.ev: LayerEvidence` (`Exact`/`Approx`/
`Hybrid``evidence.bin`/`fingerprint.bin`; see `EvidenceKind`). `Hybrid``evidence.bin`/`fingerprint.bin`; see `EvidenceKind`).
- **Matrix** = `Layer<D>.data: D` — `PersistentBitMatrix` / - **Matrix** = `TypedLayer<D>.data: D``PersistentBitMatrix` /
`PersistentCompactIntMatrix` / `PersistentSparseBitMatrix` / `()`. `PersistentCompactIntMatrix`.
Today's actual nesting, in Rust terms: Target nesting once `KmerPartition` exists:
``` ```
KmerIndex KmerIndex (obikindex)
└─ partition: KmerPartition (all N partitionspaths + metadata only) └─ (opened on demand, per i) KmerPartition (obikpartition — not yet built)
└─ (opened on demand, per i) LayeredMap<D> ← "one partition", unnamed as such └─ layers: Vec<Layer> (obilayeredmap)
└─ layers: Vec<Layer<D>> └─ Layer::Count/Presence(TypedLayer<D>)
└─ Layer<D> { mphf: MphfLayer, data: D } └─ TypedLayer<D> { mphf: MphfLayer, data: D }
├─ mphf.mphf → MPHF ├─ mphf.mphf → MPHF
├─ mphf.ev → Evidence ├─ mphf.ev → Evidence
└─ data → Matrix └─ 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<LayeredMap<D>>`, it builds `mats: Vec<Vec<Mat>>` directly — outer
index = partition `i` (via `KmerPartition`), inner index = layer `l` (via
`Layer<D>`/`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<D>`
can't, being monomorphic — see "The gap in `obilayeredmap`'s existing
cache" below).
## Major restructuring (2026-08-20): `KmerPartitions` merged into `KmerIndex` ## Major restructuring (2026-08-20): `KmerPartitions` merged into `KmerIndex`
Prompted by a direct question: why keep `KmerIndex`/`KmerPartitions` split 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 2-way not using `Layer` at all). Both remaining consumers now sit one
`Layer::open` call away from unifying onto (2) once it exists. `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<Layer>` regardless of each layer's state, states in between
included). Added one variant:
```rust
pub enum Layer {
Empty { dir: PathBuf },
Count(TypedLayer<PersistentCompactIntMatrix>),
Presence(TypedLayer<PersistentBitMatrix>),
}
```
`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 ## The problem
Reading a layer's data (MPHF + matrix) is not free: `MphfLayer::open` mmaps 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 partition's every layer once, up front, in parallel, and keeps the handles
alive for the run. 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<D>` in the table below is what's now called
`TypedLayer<D>`. `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 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? | | | lives in | scope | cached? |
|---|---|---|---| |---|---|---|---|
| `Layer<D>` | `obilayeredmap` | one layer, `D` fixed at compile time | held alive by whoever owns the `Layer`, no policy of its own | | `Layer<D>` (now `TypedLayer<D>`) | `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<D>` variants | yes, via `PartitionCache` | | `Mat` (now deleted; superseded by `obilayeredmap::Layer`) | `obikphylo::siblings::cache` | one layer, format resolved per instance from an enum of 3 `Layer<D>` variants | yes, via `PartitionCache` |
| `QueryLayer` | `obikpartition::query_layer` | one layer, `(MphfLayer, PersistentBitMatrix\|PersistentCompactIntMatrix)` pair, bypasses `Layer<D>` entirely | **no** — opened fresh inside `query_partition_with` on every call | | `QueryLayer` (unchanged, still current) | `obikindex::query_layer` (moved crates since this was written — see "Major restructuring") | one layer, `(MphfLayer, PersistentBitMatrix\|PersistentCompactIntMatrix)` pair, bypasses `TypedLayer<D>`/`Layer` entirely | **no** — opened fresh inside `query_partition_with` on every call |
`query_partition_with` is `obikmer query`'s normal query path — the one `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 most exposed to repeated cross-partition lookups — and it is still the one
no cache at all. `obikphylo` built a cache first only because sibling-annex with no cache at all. `obikphylo` built a cache first only because
construction hits the cost hardest, not because the need is sibling-specific. 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<D>` already caches correctly at the granularity `obilayeredmap::LayeredMap<D>` already caches correctly at the granularity
of one partition: `open(root)` opens every layer once, keeps `Vec<Layer<D>>` of one partition: `open(root)` opens every layer once, keeps
alive for the `LayeredMap`'s lifetime. But it is monomorphic — every layer `Vec<TypedLayer<D>>` alive for the `LayeredMap`'s lifetime. But it is
in the `Vec` must share the same concrete `D`. In practice this is false: monomorphic — every layer in the `Vec` must share the same concrete `D`.
layers in the same partition are packed independently over time (`pack In practice this was false: layers in the same partition are packed
--sparse` converts one layer's presence matrix at a time), so a partition independently over time (`pack --sparse` converts one layer's presence
can genuinely mix `PersistentBitMatrix`-backed and matrix at a time). This motivated (1) — `obilayeredmap::Layer`, done — but
`PersistentSparseBitMatrix`-backed presence layers. `LayeredMap<D>` cannot note the specific `PersistentSparseBitMatrix`-mixing scenario described
represent that mix — `Mat`'s 3-way enum exists specifically to work around here turned out to be moot: `PersistentBitMatrix` itself absorbed sparse
this gap, one crate away from where the gap actually is. 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<D>` itself is unaffected by any of this — it's still
monomorphic, still not used by `Layer`/`KmerPartition` (which bypass it
entirely, opening each `TypedLayer<D>` directly, the same way `Mat` did).
## Resource cost: mmap does not hold a file descriptor ## 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 one mmap **per genome column**, multiplying the mapping count a cache would
have to hold by `n_genomes`. have to hold by `n_genomes`.
## Layering: who owns what ## Layering: who owns what (superseded — see Definitions above)
Established in discussion, not yet coded: This section used to argue nobody owned "the collection of partitions."
That's resolved: `KmerIndex` (`obikindex`) owns it now, directly (see
- `obilayeredmap` operates within a single partition's index root; it has "Major restructuring" below). What's still genuinely unowned is *one
no notion of "partition" (confirmed independently while fixing partition's open layers* — `KmerPartition`, in the not-yet-created
`open_data`/`layer_dir` — see git history around 2026-08-20). One `obikpartition` — see "Definitions" at the top of this file for the
`LayeredMap<D>` (or its heterogeneous-`D` successor) = one partition. current, authoritative answer. Left here only so old links/references to
- Nobody currently owns "the collection of partitions" as reusable state. this heading don't 404; don't read this section for current facts.
`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.
## Direction agreed, not yet implemented ## Direction agreed, not yet implemented
1. A heterogeneous-format layer type in `obilayeredmap` (name TBD), Only (2) remains — (1) shipped as `obilayeredmap::Layer` (see "(1) done"
generalising `Mat`'s 3-way enum + auto-detection — `obilayeredmap` above). Concretely, in order:
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`.
Open before implementing: exact API shape of (1) and (2), whether `Mat` is 1. Create the `obikpartition` crate (`obikindex → obikpartition →
deleted outright or kept as a thin sibling-specific wrapper, and whether (2) obilayeredmap`, no other edges — see "Definitions" above for the exact
needs an eviction policy or can simply hold every partition open for the constraint and why).
process lifetime (revisit once the VM-mapping-count question above has a 2. `KmerPartition { layers: Vec<obilayeredmap::Layer> }` — `open`,
real number behind it for this codebase's scale). `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<KmerPartition>` 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<KmerPartition>` instead of `Vec<Vec<Layer>>`.
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<KmerPartition>` 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) ## 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`/ - `KmerPartitions` (`obikpartitionner`, at the time) gained
`layer_dir` as the single source of truth for a partition's on-disk `partition_dir`/`index_dir`/`layer_dir` as the single source of truth
layout, replacing per-module duplicated `const INDEX_SUBDIR: &str = for a partition's on-disk layout, replacing per-module duplicated
"index"` (7 copies) and ad hoc path joins — including one found `const INDEX_SUBDIR: &str = "index"` (7 copies) and ad hoc path joins —
duplicated *inside `KmerPartition` itself* (`ensure_writer` rebuilt including one found duplicated *inside the struct itself*
`part_dir`'s own logic by hand). (`ensure_writer` rebuilt `part_dir`'s own logic by hand).
- `KmerPartition` gained `partition_meta`/`n_layers`/`index_mode`, wrapping - Same struct gained `partition_meta`/`n_layers`/`index_mode`, wrapping
`obilayeredmap::meta::PartitionMeta::load` (via the existing `obilayeredmap::meta::PartitionMeta::load` (via the existing
`common::load_meta`, which also recovers indexes built before `common::load_meta`, which also recovers indexes built before
`meta.json` existed). Before this, `obikphylo` and `obikindex` imported `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 `obikphylo::siblings::family_scan::scan_layer_families` still re-derives
`index_dir` from `layer_dir.parent()` and calls `PartitionMeta::load` `index_dir` from `layer_dir.parent()` and calls `PartitionMeta::load`
itself, purely to get `.mode` for `Mat::open`. Fixing it the way the 21 itself, purely to get `.mode` for `obilayeredmap::Layer::open` (was
other call sites were fixed needs more than a 1:1 swap: `scan_layer_families` `Mat::open`, same gap, survived the `Mat` → `Layer` swap in (1) unchanged).
only receives a bare `layer_dir: &Path`, not a `(partition, part, layer)` Fixing it the way the 21 other call sites were fixed needs more than a 1:1
triple, and its single upstream source of layer paths, swap: `scan_layer_families` only receives a bare `layer_dir: &Path`, not a
`sibling_layer_dirs`, returns a flat `Vec<PathBuf>` with the partition/layer `(partition, part, layer)` triple, and its single upstream source of layer
indices already discarded. Fixing it properly means either having paths, `sibling_layer_dirs`, returns a flat `Vec<PathBuf>` with the
`sibling_layer_dirs` return `(PathBuf, IndexMode)` (or `(part, layer)`) partition/layer indices already discarded. Fixing it properly means either
pairs, or threading `&KmerPartition` + indices through instead of paths — having `sibling_layer_dirs` return `(PathBuf, IndexMode)` (or `(part,
and touching every one of `scan_layer_families`'s 8 callers (`distance.rs`, layer)`) pairs, or threading `&KmerIndex` + indices through instead of
`alignment.rs`, `cardinality.rs`, `entropy.rs` ×2, `sankoff_bundle.rs` ×2, paths (not `&KmerPartition` — that type doesn't exist yet, and once it
`stats.rs`). Left alone this round; worth doing as part of the same pass does it still won't know `IndexMode`, which lives on `KmerIndex`/
that builds (1)/(2), since those callers are exactly the sibling-annex `PartitionMeta`) — and touching every one of `scan_layer_families`'s 8
consumers (2) is meant to serve. 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.
+4
View File
@@ -183,6 +183,7 @@ impl SiblingLayerExt for obilayeredmap::Layer {
match self { match self {
obilayeredmap::Layer::Count(l) => l.iter_siblings(annex), obilayeredmap::Layer::Count(l) => l.iter_siblings(annex),
obilayeredmap::Layer::Presence(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 { match self {
obilayeredmap::Layer::Count(l) => l.iter_siblings_batch(annex, batch_size), obilayeredmap::Layer::Count(l) => l.iter_siblings_batch(annex, batch_size),
obilayeredmap::Layer::Presence(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 { match self {
obilayeredmap::Layer::Count(l) => l.iter_minorants(annex), obilayeredmap::Layer::Count(l) => l.iter_minorants(annex),
obilayeredmap::Layer::Presence(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 { match self {
obilayeredmap::Layer::Count(l) => l.iter_minorants_batch(annex, batch_size), obilayeredmap::Layer::Count(l) => l.iter_minorants_batch(annex, batch_size),
obilayeredmap::Layer::Presence(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"),
} }
} }
} }
+66 -5
View File
@@ -12,25 +12,50 @@
//! reimplement locally, minus its one sibling-specific method //! reimplement locally, minus its one sibling-specific method
//! (`iter_minorants_batch`, which stays an extension trait over there — //! (`iter_minorants_batch`, which stays an extension trait over there —
//! `obilayeredmap` has no business knowing about sibling annexes). //! `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 obicompactvec::{PersistentBitMatrix, PersistentCompactIntMatrix};
use obikseq::CanonicalKmer; use obikseq::CanonicalKmer;
use crate::error::OLMResult; 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::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 /// One layer, at any point in its life — see the module docs. Only
/// than at compile time — see the module docs. /// [`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 { 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<PersistentCompactIntMatrix>), Count(TypedLayer<PersistentCompactIntMatrix>),
Presence(TypedLayer<PersistentBitMatrix>), Presence(TypedLayer<PersistentBitMatrix>),
} }
impl Layer { 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<Self> {
std::fs::create_dir_all(dir)?;
Ok(Layer::Empty { dir: dir.to_owned() })
}
/// Open one layer, auto-detecting count vs. presence from what is /// Open one layer, auto-detecting count vs. presence from what is
/// actually on disk (`counts/` present and wanted, else presence) — the /// actually on disk (`counts/` present and wanted, else presence) — the
/// single source of truth every caller that opens a layer's own matrix /// single source of truth every caller that opens a layer's own matrix
@@ -46,10 +71,40 @@ impl Layer {
TypedLayer::<PersistentBitMatrix>::open(layer_dir, mode).map(Layer::Presence) TypedLayer::<PersistentBitMatrix>::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 { pub fn content(&self) -> LayerContent {
match self { match self {
Layer::Count(_) => LayerContent::Count, Layer::Count(_) => LayerContent::Count,
Layer::Presence(_) => LayerContent::Presence, Layer::Presence(_) => LayerContent::Presence,
Layer::Empty { .. } => panic!("Layer::content() called on an Empty layer"),
} }
} }
@@ -57,6 +112,7 @@ impl Layer {
match self { match self {
Layer::Count(l) => l.evidence_kind(), Layer::Count(l) => l.evidence_kind(),
Layer::Presence(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 { match self {
Layer::Count(l) => l.n(), Layer::Count(l) => l.n(),
Layer::Presence(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 { match self {
Layer::Count(l) => l.find_slot(kmer), Layer::Count(l) => l.find_slot(kmer),
Layer::Presence(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 { match self {
Layer::Count(l) => l.index_batch(kmers), Layer::Count(l) => l.index_batch(kmers),
Layer::Presence(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 { match self {
Layer::Count(l) => l.n_cols(), Layer::Count(l) => l.n_cols(),
Layer::Presence(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)); o.extend(c.iter().map(|&v| v != 0));
} }
} }
Layer::Empty { .. } => panic!("Layer::fill_sub_matrix_carries() called on an Empty layer"),
} }
} }
} }
+1 -1
View File
@@ -17,7 +17,7 @@ use crate::mphf_layer::MphfLayer;
pub(crate) use crate::mphf_layer::UNITIGS_FILE; pub(crate) use crate::mphf_layer::UNITIGS_FILE;
pub(crate) const COUNTS_DIR: &str = "counts"; pub(crate) const COUNTS_DIR: &str = "counts";
const PRESENCE_DIR: &str = "presence"; pub(crate) const PRESENCE_DIR: &str = "presence";
// ── Trait ───────────────────────────────────────────────────────────────────── // ── Trait ─────────────────────────────────────────────────────────────────────