From 5a9181748823dd82d405df406662143e8446f0e2 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Thu, 20 Aug 2026 06:27:16 +0200 Subject: [PATCH] centralize layer path construction and data loading Replace manual directory formatting and inline matrix opening with the newly introduced `layer_dir` and `open_data` helpers from the `obilayeredmap` crate. This standardizes filesystem access, encapsulates layer naming conventions, and simplifies error handling without altering computational behavior or test suites. --- src/obikindex/src/stats.rs | 14 ++++++------- src/obikpartitionner/src/distance.rs | 27 ++++++++++--------------- src/obilayeredmap/src/layer.rs | 30 +++++++++++++++++++++++++++- src/obilayeredmap/src/lib.rs | 2 +- src/obilayeredmap/src/map.rs | 6 +----- 5 files changed, 49 insertions(+), 30 deletions(-) diff --git a/src/obikindex/src/stats.rs b/src/obikindex/src/stats.rs index 434af17a..da876286 100644 --- a/src/obikindex/src/stats.rs +++ b/src/obikindex/src/stats.rs @@ -149,21 +149,21 @@ impl KmerIndex { .unwrap_or(0); for l in 0..n_layers { - let layer_dir = index_dir.join(format!("layer_{l}")); - if !layer_dir.exists() { continue; } + let this_layer_dir = obilayeredmap::layer_dir(&index_dir, l); + if !this_layer_dir.exists() { continue; } - n_kmers += LayerMeta::load(&layer_dir).map(|m| m.n).unwrap_or(0); + n_kmers += LayerMeta::load(&this_layer_dir).map(|m| m.n).unwrap_or(0); let mat: Box = - if layer_dir.join("counts").exists() - && !layer_dir.join("presence").exists() + if this_layer_dir.join("counts").exists() + && !this_layer_dir.join("presence").exists() { - match PersistentCompactIntMatrix::open(&layer_dir) { + match obilayeredmap::open_data::(&index_dir, l) { Ok(m) => Box::new(m), Err(_) => continue, } } else { - match PersistentBitMatrix::open(&layer_dir) { + match obilayeredmap::open_data::(&index_dir, l) { Ok(m) => Box::new(m), Err(_) => continue, } diff --git a/src/obikpartitionner/src/distance.rs b/src/obikpartitionner/src/distance.rs index f25a45f9..3cc33aeb 100644 --- a/src/obikpartitionner/src/distance.rs +++ b/src/obikpartitionner/src/distance.rs @@ -1,17 +1,12 @@ use obicompactvec::{PersistentBitMatrix, PersistentCompactIntMatrix}; -use obilayeredmap::LayeredStore; -use obiskio::{SKError, SKResult}; +use obilayeredmap::{layer_dir, open_data, LayeredStore}; +use obiskio::SKResult; +use crate::common::{load_meta, olm_to_sk}; use crate::partition::KmerPartition; const INDEX_SUBDIR: &str = "index"; -fn probe_n_layers(index_dir: &std::path::Path) -> usize { - let mut n = 0; - while index_dir.join(format!("layer_{n}")).exists() { n += 1; } - n -} - impl KmerPartition { /// Open all count matrices for partition `part`, one per layer. /// Layers without a `counts/` directory are skipped. @@ -20,11 +15,11 @@ impl KmerPartition { if !index_dir.exists() { return Ok(LayeredStore::new(vec![])); } - let matrices = (0..probe_n_layers(&index_dir)) + let n_layers = load_meta(&index_dir, "distance")?.n_layers; + let matrices = (0..n_layers) .filter_map(|l| { - let layer_dir = index_dir.join(format!("layer_{l}")); - layer_dir.join("counts").exists() - .then(|| PersistentCompactIntMatrix::open(&layer_dir).map_err(SKError::Io)) + layer_dir(&index_dir, l).join("counts").exists() + .then(|| open_data(&index_dir, l).map_err(|e| olm_to_sk(e, "distance"))) }) .collect::>>()?; Ok(LayeredStore::new(matrices)) @@ -37,11 +32,11 @@ impl KmerPartition { if !index_dir.exists() { return Ok(LayeredStore::new(vec![])); } - let matrices = (0..probe_n_layers(&index_dir)) + let n_layers = load_meta(&index_dir, "distance")?.n_layers; + let matrices = (0..n_layers) .filter_map(|l| { - let layer_dir = index_dir.join(format!("layer_{l}")); - layer_dir.join("presence").exists() - .then(|| PersistentBitMatrix::open(&layer_dir).map_err(SKError::Io)) + layer_dir(&index_dir, l).join("presence").exists() + .then(|| open_data(&index_dir, l).map_err(|e| olm_to_sk(e, "distance"))) }) .collect::>>()?; Ok(LayeredStore::new(matrices)) diff --git a/src/obilayeredmap/src/layer.rs b/src/obilayeredmap/src/layer.rs index bf53fae8..f0b22caa 100644 --- a/src/obilayeredmap/src/layer.rs +++ b/src/obilayeredmap/src/layer.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use obicompactvec::{ BinaryMatrix, @@ -27,6 +27,34 @@ pub trait LayerData: Sized { fn read(&self, slot: usize) -> Self::Item; } +/// Layer directory path for layer `i` within a partition's index root — the +/// single source of truth for the on-disk `layer_N` naming convention +/// (mirrors what `LayeredMap::open`/`push_layer` already use internally). +/// +/// `obilayeredmap` operates within a single partition's index root; it has +/// no notion of "partition" at all. Turning a partition number into that +/// root is `obikpartitionner::KmerPartition::part_dir`'s job, one layer up +/// — callers here only ever name a layer *number*, never build the path +/// themselves. +pub fn layer_dir(root: &Path, i: usize) -> PathBuf { + root.join(format!("layer_{i}")) +} + +/// Opens layer `i`'s data only, skipping the MPHF — for callers that only +/// need matrix-level operations (distance traits, column weights, group +/// filters, sub-matrix extraction) and never look up a kmer for this layer. +/// `Layer::open` always pays for the MPHF too (and doesn't expose `data` +/// once open), so it's the wrong tool for these; this is the other half of +/// the same `D::open(layer_dir)` call, without the MPHF alongside it. +/// +/// Takes `(root, i)`, not a pre-built path: the caller names a layer +/// *number* within a partition it already knows the root of, the same +/// vocabulary as [`layer_dir`] and `LayeredMap` — never the `layer_N` +/// naming convention itself, which stays private to this crate. +pub fn open_data(root: &Path, i: usize) -> OLMResult { + D::open(&layer_dir(root, i)) +} + impl LayerData for () { type Item = (); fn open(_layer_dir: &Path) -> OLMResult { Ok(()) } diff --git a/src/obilayeredmap/src/lib.rs b/src/obilayeredmap/src/lib.rs index 9c7f9975..103035eb 100644 --- a/src/obilayeredmap/src/lib.rs +++ b/src/obilayeredmap/src/lib.rs @@ -8,7 +8,7 @@ pub mod meta; pub(crate) mod mphf_layer; pub use error::{OLMError, OLMResult}; -pub use layer::{Hit, Layer, LayerData}; +pub use layer::{layer_dir, open_data, Hit, Layer, LayerData}; pub use layered_store::LayeredStore; pub use map::LayeredMap; pub use meta::{IndexMode, PartitionMeta}; diff --git a/src/obilayeredmap/src/map.rs b/src/obilayeredmap/src/map.rs index 31cf20c2..9a8971de 100644 --- a/src/obilayeredmap/src/map.rs +++ b/src/obilayeredmap/src/map.rs @@ -7,7 +7,7 @@ use obikseq::CanonicalKmer; use obiskio::{UnitigFileWriter, DEFAULT_BLOCK_BITS}; use crate::error::OLMResult; -use crate::layer::{Hit, Layer, LayerData}; +use crate::layer::{layer_dir, Hit, Layer, LayerData}; use crate::meta::{IndexMode, PartitionMeta}; /// Layered kmer index for a single partition. @@ -97,10 +97,6 @@ impl LayeredMap { } } -fn layer_dir(root: &Path, i: usize) -> PathBuf { - root.join(format!("layer_{i}")) -} - #[cfg(test)] #[path = "tests/map.rs"] mod tests;