refactor: merge KmerPartitions into KmerIndex and rename obikpartition
Consolidates partition logic, metadata storage, and layer management directly into KmerIndex. Renames obikpartitionner to obikpartition, retaining only PartitionRouter for superkmer routing. Removes intermediate .partition() accessors in favor of direct methods on the index and updates PartitionCache::build to accept &KmerIndex directly. Derives n_partitions from config.n_bits and consolidates k-mer/minimizer sizes into IndexMeta.config. Fixes a regression where PartitionRouter::open incorrectly defaulted to closed.
This commit is contained in:
@@ -69,6 +69,79 @@ 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`
|
||||
|
||||
Prompted by a direct question: why keep `KmerIndex`/`KmerPartitions` split
|
||||
when, one level down, `KmerPartitions` is going to directly hold
|
||||
`Vec<KmerPartition>` rather than being split again into
|
||||
"collection-holder" + "collection"? Investigating the actual justification
|
||||
("`KmerPartitions` has an independent lifecycle, used before an index
|
||||
exists") turned out to be **false** — `KmerPartitions::create` was called
|
||||
in exactly one place, inside `KmerIndex::create`, and every
|
||||
`open_with_config` reopen outside `KmerIndex`'s own constructors was a
|
||||
redundant re-derivation of a `KmerPartitions` already reachable via
|
||||
`index.partition()` (the exact kind of duplication this whole doc has been
|
||||
tracking). Once that was gone, so was the reason to keep them separate.
|
||||
|
||||
Second correction, from the same conversation: `obikpartitionner` had
|
||||
accumulated query/merge/select/rebuild/dump/distance logic that has
|
||||
nothing to do with partitioning super-kmers — it operates on *layers*,
|
||||
which don't exist yet at the phase `obikpartitionner` is actually
|
||||
responsible for (scatter → dereplicate → count, all pre-layer). That
|
||||
logic moved to `obikindex`, which already depends on `obilayeredmap` and
|
||||
never needed `obikpartitionner` for it. No crate-dependency inversion was
|
||||
needed — `obikindex → obikpartitionner` stays the same direction as before.
|
||||
|
||||
**Result:**
|
||||
- `obikpartitionner` (renamed back from `obikpartition`) now contains only
|
||||
`PartitionRouter` (superkmer routing: `write`/`write_batch`/`flush`/
|
||||
`close`, `dereplicate`, `count_kmer`, `KmerSpectrum`) and the
|
||||
`partition_dir(root, i)` naming primitive both `PartitionRouter` and
|
||||
`KmerIndex` build on. `KmerPartitions` no longer exists as a type.
|
||||
- `KmerIndex` (`obikindex`) absorbed `KmerPartitions`'s read-side entirely:
|
||||
`partition_dir`/`index_dir`/`layer_dir`/`partition_meta`/`n_layers`/
|
||||
`partition_mode`/`n_partitions` (the last now derived from
|
||||
`2^config.n_bits`, no longer a stored, independently-set duplicate field
|
||||
— `kmer_size`/`minimizer_size` used to be double-stored, in both
|
||||
`KmerPartitions` and `IndexMeta.config`, a latent-drift risk flagged
|
||||
earlier in this doc; now single-sourced from `IndexMeta.config`). Seven
|
||||
whole files moved from `obikpartitionner` into `obikindex` verbatim as
|
||||
`impl KmerIndex` blocks, kept as separate files (not merged into
|
||||
existing same-topic files): `index_layer.rs`, `query_layer.rs`,
|
||||
`merge_layer/`, `select_layer.rs`, `rebuild_layer.rs`, `dump_layer.rs`,
|
||||
plus `distance.rs`'s `count_store`/`presence_store` (renamed
|
||||
`matrix_store.rs` to avoid colliding with `obikindex`'s own pre-existing
|
||||
`distance.rs`), and their shared support (`common.rs`'s `load_meta`/
|
||||
`olm_to_sk`, `filter.rs`, `graph_pipeline.rs`).
|
||||
- `obikphylo::siblings::cache::PartitionCache::build` now takes `&KmerIndex`
|
||||
directly instead of a separately-opened `&KmerPartitions` — this deleted
|
||||
the redundant-reopen pattern at all 8 call sites
|
||||
(`alignment`/`build`/`cardinality`/`distance`/`entropy`×2/
|
||||
`sankoff_bundle`/`stats`), the same bug flagged earlier in this
|
||||
conversation as a side effect of investigating the false "independent
|
||||
lifecycle" claim.
|
||||
- `KmerIndex::partition()`/`partition_mut()` are gone; `scatter()`
|
||||
(`obikmer`) and any write-side code get a transient `PartitionRouter` via
|
||||
`KmerIndex::partition_router()`.
|
||||
- A real bug caught by the test suite during this move:
|
||||
`PartitionRouter::open` initially defaulted to `closed: true` (inherited
|
||||
from `KmerPartitions::open_with_config`'s old read-only-reopen
|
||||
semantics), which broke every write through a router obtained via
|
||||
`partition_router()`. Fixed — `PartitionRouter` is exclusively a
|
||||
write/processing tool now, so `open` always starts open.
|
||||
|
||||
Full workspace test suite green (0 failed) after, including all 27
|
||||
`obikphylo::siblings` tests.
|
||||
|
||||
Still open: (1)/(2) themselves — `AnyLayer` (or whatever it ends up named;
|
||||
`AnyLayer` was rejected as a placeholder, no replacement chosen yet) and
|
||||
`KmerPartition` (singular, one partition's open layers) are not built yet.
|
||||
`obikphylo::siblings::cache::{Mat, PartitionCache}` and
|
||||
`obikpartitionner::query_layer` (now `obikindex::query_layer`)'s
|
||||
`QueryLayer` still each independently bundle MPHF+matrix — unchanged by
|
||||
this restructuring, which was purely about *where* code lives, not about
|
||||
building the heterogeneous-layer cache itself.
|
||||
|
||||
## The problem
|
||||
|
||||
Reading a layer's data (MPHF + matrix) is not free: `MphfLayer::open` mmaps
|
||||
|
||||
Reference in New Issue
Block a user