Extract k-mer counting logic into a dedicated counter module
Decouple k-mer counting from the partitioner by introducing a new `Counter` struct. The module exposes a fluent builder API with optional partial file retention, executes partition processing in parallel via Rayon with memory-aware chunk sizing, and integrates thread-safe progress callbacks. Update all callers to use the new counter, simplify test pipelines by removing serialization overhead, and clarify algorithm separation in module documentation.
This commit is contained in:
@@ -51,7 +51,10 @@ turned out to be bigger than `KmerPartition` alone: `Layer`'s own
|
||||
constructors don't self-name either. Full redesign of both, agreed in
|
||||
detail, session ended (budget) before implementation — see "(5) design
|
||||
agreed" below; **read it before touching `KmerPartition`/`Layer`
|
||||
signatures**, the shape is fully specified. Earlier mix-up, for
|
||||
signatures**, the shape is fully specified. (6) done — `Counter`, a third
|
||||
algorithm, extracted from `PartitionRouter` the same way `Dereplicator`
|
||||
was in (4); (5) itself still not implemented, still first on the "order of
|
||||
remaining work" list — see "(6) done" below. 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
|
||||
@@ -772,6 +775,82 @@ itself or a new crate.
|
||||
examples were judged not enough to be sure of the shape (`Fn+Sync` vs
|
||||
`FnMut` callback bound already diverged between the two that exist).
|
||||
|
||||
## (6) done (2026-08-21): `Counter` — third algorithm, extracted the same way as `Dereplicator`
|
||||
|
||||
Between (5) and this, the user did a session of their own crate
|
||||
restructuring (see the two "Superseded" notes at the top of this file):
|
||||
`obikpartition`/`obilayeredmap` folded into `obikindex` as submodules
|
||||
(`obikindex::partition`, `obikindex::layer`), and `obikpartitionner`/
|
||||
`obikderep` merged into one sibling crate, `obikindexer`, holding
|
||||
`obikindexer::algorithms::{partitionner, dereplicator}`. (5)'s design
|
||||
(`Layer`/`KmerPartition` self-naming by number, `PartitionRouter`'s
|
||||
`&mut` → `&` fix) was **not** part of that — pure crate/module packaging,
|
||||
confirmed by reading the actual code (`Layer::open`/`create` still take an
|
||||
external `dir: &Path`, `KmerPartition` still eagerly opens all layers,
|
||||
`PartitionRouter` still holds `&mut KmerIndex`). (5) remains exactly as
|
||||
specified, not yet implemented.
|
||||
|
||||
This step: `count_kmer` (still living on `PartitionRouter`, per (4)'s own
|
||||
"still not done" note) extracted into `obikindexer::algorithms::counter::
|
||||
Counter`, mirroring `Dereplicator` exactly — third data point for the
|
||||
eventual `obikalgorithm` trait, still not extracted (still only 3 examples
|
||||
with 2 different callback bounds; holding off per (5)'s "order of
|
||||
remaining work").
|
||||
|
||||
```rust
|
||||
pub struct Counter<'a> {
|
||||
index: &'a KmerIndex,
|
||||
n_partitions: usize,
|
||||
keep_partial: bool,
|
||||
}
|
||||
|
||||
impl<'a> Counter<'a> {
|
||||
pub fn new(index: &'a KmerIndex) -> Self;
|
||||
pub fn keep_partial(mut self, v: bool) -> Self; // setter, mirrors PartitionRouter's style; defaults to false
|
||||
pub fn run(&self, on_progress: Option<impl Fn(Progress) + Sync>) -> SKResult<KmerSpectrum>;
|
||||
}
|
||||
```
|
||||
|
||||
Same shape as `Dereplicator` throughout: `Fn(Progress) + Sync` (not
|
||||
`FnMut`) since counting is also a parallel `par_iter` over partitions, an
|
||||
`AtomicU64` position counter incremented from inside the parallel closure
|
||||
so progress reports arrive in real time rather than bursting at the end
|
||||
once `.collect()` finishes, `total: Some(n_partitions)` (known up front).
|
||||
`KmerSpectrum` (the `{f0, f1, counts}` aggregate) moved from
|
||||
`partitionner::router` to `counter`, since it's `Counter::run`'s return
|
||||
value now, not `PartitionRouter`'s. `count.rs`/`kmer_sort.rs` moved
|
||||
verbatim from `partitionner/` to `counter/` (unchanged bodies — only
|
||||
`count_kmer` itself, `KmerSpectrum`, and the imports they pulled in were
|
||||
removed from `router.rs`).
|
||||
|
||||
One divergence from `Dereplicator`: a `keep_partial` setter exists (no
|
||||
equivalent on `Dereplicator`, which has no setters at all) — a real,
|
||||
already-present parameter (`keep_intermediate` at the CLI), not a
|
||||
speculative addition.
|
||||
|
||||
`count_kmer`'s three former callers (`obikmer::cmd::index`, `obikphylo`'s
|
||||
test harness, `obikindexer::algorithms::partitionner`'s own
|
||||
`pipeline_counts` test helper) all updated to `Counter::new(&idx).
|
||||
run(...)` — the last one simplified further: it used to read back
|
||||
`kmer_spectrum_raw.json` from disk after calling `count_partition`
|
||||
directly (white-box), now it just uses the `KmerSpectrum` `Counter::run`
|
||||
already returns.
|
||||
|
||||
Full workspace suite green (`cargo check --workspace --all-targets` +
|
||||
`cargo test --workspace`, exit code 0), plus an end-to-end CLI smoke test
|
||||
against real FASTA data (scatter → dereplicate → count → index-build →
|
||||
query) — required every time per (3)'s lesson, and it earned its keep
|
||||
again: the very first smoke-test query returned zero matches, which
|
||||
looked like a regression until traced to the query sequence itself being
|
||||
low-complexity ("GGCCCCCCACG", six same-base runs) and rejected by
|
||||
*query's own* default entropy threshold — nothing to do with this change.
|
||||
Re-tested with a different substring, confirmed working (kmer found,
|
||||
count matched the index).
|
||||
|
||||
Still not done: (5) (`Layer`/`KmerPartition` redesign, `PartitionRouter`'s
|
||||
`&mut`→`&`), the future cache crate, `build_layers` (still a `KmerIndex`
|
||||
inherent method, not an algorithm), and `obikalgorithm` itself.
|
||||
|
||||
## The problem
|
||||
|
||||
Reading a layer's data (MPHF + matrix) is not free: `MphfLayer::open` mmaps
|
||||
|
||||
Reference in New Issue
Block a user