rename KmerPartition to KmerPartitions and update Mat enum
Rename the KmerPartition type to KmerPartitions across obikindex, obikpartitionner, and obikphylo/siblings to reflect an updated data model. Update the Mat enum in siblings/cache.rs to add a SparsePresence variant and simplify opening logic by delegating sparse versus dense detection to PersistentBitMatrix. Apply consistent code formatting, import reordering, and multi-line refactoring throughout the affected modules.
This commit is contained in:
@@ -5,6 +5,70 @@ 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.
|
||||
|
||||
## 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.
|
||||
|
||||
- **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)** =
|
||||
`obikpartitionner::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_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<Layer<D>>`
|
||||
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<D>` — `{ root, meta: PartitionMeta, layers:
|
||||
Vec<Layer<D>> }` — 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<D>` directly.
|
||||
- **Layer** = `obilayeredmap::Layer<D>` — `{ mphf: MphfLayer, data: D }`.
|
||||
- **MPHF** = `MphfLayer.mphf: MemCase<MphfEps>` — kmer → slot.
|
||||
- **Evidence** = `MphfLayer.ev: LayerEvidence` (`Exact`/`Approx`/
|
||||
`Hybrid` — `evidence.bin`/`fingerprint.bin`; see `EvidenceKind`).
|
||||
- **Matrix** = `Layer<D>.data: D` — `PersistentBitMatrix` /
|
||||
`PersistentCompactIntMatrix` / `PersistentSparseBitMatrix` / `()`.
|
||||
|
||||
Today's actual nesting, in Rust terms:
|
||||
|
||||
```
|
||||
KmerIndex
|
||||
└─ partition: KmerPartition (all N partitions — paths + metadata only)
|
||||
└─ (opened on demand, per i) LayeredMap<D> ← "one partition", unnamed as such
|
||||
└─ layers: Vec<Layer<D>>
|
||||
└─ Layer<D> { 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<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).
|
||||
|
||||
## The problem
|
||||
|
||||
Reading a layer's data (MPHF + matrix) is not free: `MphfLayer::open` mmaps
|
||||
@@ -217,13 +281,18 @@ storage decisions, now a *third* copy of the same logic to keep in sync)
|
||||
about the code. Lesson: for a `pub enum` whose variant list matters,
|
||||
read the definition unfiltered, don't grep for the variant names you
|
||||
expect to find.
|
||||
- One real consequence of that same correction: `obikphylo::siblings::
|
||||
cache::Mat::SparsePresence(Layer<PersistentSparseBitMatrix>)` looks
|
||||
redundant now — `Mat::Presence(Layer<PersistentBitMatrix>)` alone
|
||||
would already handle sparse layers transparently, since
|
||||
`PersistentBitMatrix` absorbs `Sparse` internally. Likely `Mat` predates
|
||||
`PersistentBitMatrix` growing native sparse support. Signalled, not
|
||||
removed — no mandate to touch `obikphylo` for this.
|
||||
- One real consequence of that same correction, **fixed (2026-08-20)**:
|
||||
`obikphylo::siblings::cache::Mat::SparsePresence(Layer<
|
||||
PersistentSparseBitMatrix>)` was redundant — `Mat::Presence(Layer<
|
||||
PersistentBitMatrix>)` alone already handles sparse layers
|
||||
transparently, since `PersistentBitMatrix` absorbs `Sparse` internally.
|
||||
Removed the variant, the `presence/is_multi.prsb` probe in `Mat::open`
|
||||
(now just opens `Layer::<PersistentBitMatrix>` unconditionally for the
|
||||
non-count case — sparse-vs-dense is `PersistentBitMatrix::open`'s own
|
||||
concern), and every now-single-armed match in `find_slot`/`index_batch`/
|
||||
`iter_minorants_batch`/`n_cols`/`fill_sub_matrix_carries`. Full
|
||||
workspace test suite green after, including the 27 `obikphylo::siblings`
|
||||
tests that exercise `pack_matrices(true)`/sparse through `Mat`.
|
||||
|
||||
## Remaining instance of the PartitionMeta-encapsulation problem
|
||||
|
||||
|
||||
Reference in New Issue
Block a user