From 93f4063f6f391e99602970cdfc84bd163109e3a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 01:31:57 +0000 Subject: [PATCH 01/13] Rebase ZarrV3 save-system rework onto current master Rebases the zarrs-save-rework branch (6 save-system commits) onto master. Master had since split ext::secondary into per-primary submodules (#254) and made the secondary lift entry points return Result (#241, #242), so the conflicts were confined to ext/src/secondary.rs. Resolution: - Adopt the store-based save API (store.read/write/exists/delete) in the shared SecondaryHomotopy / SecondaryLift code while preserving master's Result-returning try_compute_homotopy_step. - Drop the stale monolithic duplicates of SecondaryResolution, SecondaryResolutionHomomorphism, and SecondaryChainHomotopy; those types now live in submodules under resolution.rs / chain_homotopy.rs / resolution_homomorphism.rs. - Remove the per-new() SaveKind::secondary_data() create_dir loops from the moved constructors: the reworked store creates its shard arrays lazily on first write, and create_dir no longer exists on SaveKind. Dropped from the original branch: the throwaway "experimental rayon build" commit (patched rayon to a personal fork) and the earlier "tracing to parallel guard" commit (master ships a superior version). All 7 save_load_resolution tests and the ext lib unit tests pass. --- ext/Cargo.toml | 14 + ext/crates/fp/src/matrix/quasi_inverse.rs | 91 +- ext/crates/fp/src/vector/fp_wrapper/mod.rs | 7 +- ext/src/chain_complex/chain_homotopy.rs | 56 +- ext/src/chain_complex/mod.rs | 14 - ext/src/nassau.rs | 374 ++--- ext/src/resolution.rs | 210 ++- ext/src/resolution_homomorphism.rs | 70 +- ext/src/save.rs | 1569 +++++++++++++++----- ext/src/secondary.rs | 119 +- ext/tests/save_load_resolution.rs | 176 +-- 11 files changed, 1570 insertions(+), 1130 deletions(-) diff --git a/ext/Cargo.toml b/ext/Cargo.toml index 8b3919aba8..bb1128e18a 100644 --- a/ext/Cargo.toml +++ b/ext/Cargo.toml @@ -23,6 +23,8 @@ sseq = { path = "crates/sseq", default-features = false } adler = "1" anyhow = "1.0.98" +bitcode = { version = "0.6", features = ["serde"] } +serde = { version = "1", features = ["derive"] } byteorder = "1.5.0" dashmap = "6.1.0" itertools = { version = "0.14.0", default-features = false, features = [ @@ -35,6 +37,18 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } zstd = { version = "0.13.3", optional = true } +# Zarrs is declared twice with target-specific feature sets. On native targets we enable the +# `filesystem` feature so the save system persists to disk; on `wasm32-unknown-unknown` we +# fall back to an in-memory store (see `src/save.rs`). The `filesystem` feature pulls in +# `positioned-io::RandomAccessFile`, which is gated to `cfg(any(windows, unix))` and doesn't +# compile on wasm, and the `zstd` feature pulls in `zstd-sys`, whose C build expects POSIX +# symbols like `qsort_r` that wasm's libc shim doesn't expose — so both are dropped on wasm. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +zarrs = { version = "0.23", default-features = false, features = ["filesystem", "sharding", "crc32c", "zstd"] } + +[target.'cfg(target_arch = "wasm32")'.dependencies] +zarrs = { version = "0.23", default-features = false, features = ["sharding", "crc32c"] } + [target.'cfg(unix)'.dependencies] ctrlc = { version = "3", features = ["termination"] } diff --git a/ext/crates/fp/src/matrix/quasi_inverse.rs b/ext/crates/fp/src/matrix/quasi_inverse.rs index 833f3ef9f8..b7b2f0e814 100644 --- a/ext/crates/fp/src/matrix/quasi_inverse.rs +++ b/ext/crates/fp/src/matrix/quasi_inverse.rs @@ -1,13 +1,12 @@ use std::io; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; -use itertools::Itertools; use serde::{Deserialize, Serialize}; use super::Matrix; use crate::{ prime::ValidPrime, - vector::{FpSlice, FpSliceMut, FpVector}, + vector::{FpSlice, FpSliceMut}, }; /// Given a matrix M, a quasi-inverse Q is a map from the co-domain to the domain such that xQM = x @@ -75,44 +74,6 @@ impl QuasiInverse { }) } - /// Given a data file containing a quasi-inverse, apply it to all the vectors in `input` - /// and write the results to the corresponding vectors in `results`. This reads in the - /// quasi-inverse row by row to minimize memory usage. - pub fn stream_quasi_inverse( - p: ValidPrime, - data: &mut impl io::Read, - results: &mut [T], - inputs: &[S], - ) -> io::Result<()> - where - for<'a> &'a mut T: Into>, - for<'a> &'a S: Into>, - { - let source_dim = data.read_u64::()? as usize; - let target_dim = data.read_u64::()? as usize; - let _image_dim = data.read_u64::()? as usize; - - let image = Matrix::read_pivot(target_dim, data)?; - let mut v = FpVector::new(p, source_dim); - - assert_eq!(results.len(), inputs.len()); - for result in &mut *results { - assert_eq!(result.into().as_slice().len(), source_dim); - } - - for (i, r) in image.into_iter().enumerate() { - if r < 0 { - continue; - } - - v.update_from_bytes(data)?; - for (input, result) in inputs.iter().zip_eq(&mut *results) { - result.into().add(v.as_slice(), input.into().entry(i)); - } - } - Ok(()) - } - pub fn preimage(&self) -> &Matrix { &self.preimage } @@ -148,53 +109,3 @@ impl QuasiInverse { } } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_stream_qi() { - let p = ValidPrime::new(2); - let qi = QuasiInverse { - image: Some(vec![0, -1, 1, -1, 2, 3]), - preimage: Matrix::from_vec( - p, - &[ - vec![1, 0, 1, 1], - vec![1, 1, 0, 0], - vec![0, 1, 0, 1], - vec![1, 1, 1, 0], - ], - ), - }; - let v0 = FpVector::from_slice(p, &[1, 1, 0, 0, 1, 0]); - let v1 = FpVector::from_slice(p, &[0, 0, 1, 0, 1, 1]); - - let mut out0 = FpVector::new(p, 4); - let mut out1 = FpVector::new(p, 4); - - let mut cursor = io::Cursor::new(Vec::::new()); - qi.to_bytes(&mut cursor).unwrap(); - cursor.set_position(0); - - QuasiInverse::stream_quasi_inverse( - p, - &mut cursor, - &mut [out0.as_slice_mut(), out1.as_slice_mut()], - &[v0.as_slice(), v1.as_slice()], - ) - .unwrap(); - - let mut bench0 = FpVector::new(p, 4); - let mut bench1 = FpVector::new(p, 4); - - qi.apply(bench0.as_slice_mut(), 1, v0.as_slice()); - qi.apply(bench1.as_slice_mut(), 1, v1.as_slice()); - - assert_eq!(out0, bench0, "{out0} != {bench0}"); - assert_eq!(out1, bench1, "{out1} != {bench1}"); - - assert_eq!(cursor.position() as usize, cursor.get_ref().len()); - } -} diff --git a/ext/crates/fp/src/vector/fp_wrapper/mod.rs b/ext/crates/fp/src/vector/fp_wrapper/mod.rs index 112760002a..622b831661 100644 --- a/ext/crates/fp/src/vector/fp_wrapper/mod.rs +++ b/ext/crates/fp/src/vector/fp_wrapper/mod.rs @@ -112,8 +112,11 @@ impl FpVector { v } - // Convenient for some matrix methods - pub(crate) fn num_limbs(p: ValidPrime, len: usize) -> usize { + /// The number of `Limb`s required to store an [`FpVector`] of length + /// `len` over `F_p`. Useful for sizing a buffer that holds the + /// little-endian byte serialization of an `FpVector` (each limb is 8 + /// bytes). + pub fn num_limbs(p: ValidPrime, len: usize) -> usize { Fp::new(p).number(len) } diff --git a/ext/src/chain_complex/chain_homotopy.rs b/ext/src/chain_complex/chain_homotopy.rs index c0ec0fd0d2..13f269c994 100644 --- a/ext/src/chain_complex/chain_homotopy.rs +++ b/ext/src/chain_complex/chain_homotopy.rs @@ -29,9 +29,13 @@ pub struct ChainHomotopy< left: Arc>, right: Arc>, lock: Mutex<()>, + /// Save directory for this homotopy and any secondary lift built from it. + /// Set to a `homotopies/{left}__{right}` subgroup of the source's save_dir + /// when both `left` and `right` are named, `None` otherwise so that + /// homotopies between anonymous homs don't pollute the store. + save_dir: SaveDirectory, /// Homotopies, indexed by the filtration of the target of f - g. homotopies: OnceBiVec>>, - save_dir: SaveDirectory, } impl< @@ -44,23 +48,19 @@ impl< left: Arc>, right: Arc>, ) -> Self { - let save_dir = if left.source.save_dir().is_some() - && !left.name().is_empty() + assert!(Arc::ptr_eq(&left.target, &right.source)); + let save_dir = if !left.name().is_empty() && !right.name().is_empty() + && let Some(parent) = left.source.save_dir().store() { - let mut save_dir = left.source.save_dir().clone(); - save_dir.push(format!("massey/{},{}/", left.name(), right.name())); - - SaveKind::ChainHomotopy - .create_dir(save_dir.write().unwrap()) - .unwrap(); - - save_dir + SaveDirectory::Store( + parent + .subgroup(&format!("homotopies/{}__{}", left.name(), right.name())) + .expect("Failed to create homotopies subgroup"), + ) } else { SaveDirectory::None }; - - assert!(Arc::ptr_eq(&left.target, &right.source)); Self { homotopies: OnceBiVec::new((left.shift + right.shift).s() - 1), left, @@ -178,16 +178,13 @@ impl< return self.homotopies[source.s()].add_generators_from_rows_ooo(source.t(), outputs); } - if let Some(dir) = self.save_dir.read() - && let Some(mut f) = self - .left - .source - .save_file(SaveKind::ChainHomotopy, source) - .open_file(dir.to_owned()) + if let Some(store) = self.save_dir.store() + && let Some(data) = store.read(SaveKind::ChainHomotopy, source).unwrap() { + let mut cursor = &data[..]; let mut outputs = Vec::with_capacity(num_gens); for _ in 0..num_gens { - outputs.push(FpVector::from_bytes(p, target_dim, &mut f).unwrap()); + outputs.push(FpVector::from_bytes(p, target_dim, &mut cursor).unwrap()); } return self.homotopies[source.s()].add_generators_from_rows_ooo(source.t(), outputs); } @@ -263,15 +260,12 @@ impl< &scratches, )); - if let Some(dir) = self.save_dir.write() { - let mut f = self - .left - .source - .save_file(SaveKind::ChainHomotopy, source) - .create_file(dir.to_owned(), false); + if let Some(store) = self.save_dir.store() { + let mut buf = Vec::new(); for row in &outputs { - row.to_bytes(&mut f).unwrap(); + row.to_bytes(&mut buf).unwrap(); } + store.write(SaveKind::ChainHomotopy, source, &buf).unwrap(); } self.homotopies[source.s()].add_generators_from_rows_ooo(source.t(), outputs) } @@ -306,7 +300,7 @@ pub(crate) mod secondary { use crate::{ chain_complex::FreeChainComplex, resolution_homomorphism::ResolutionHomomorphism, - save::{SaveDirectory, SaveKind}, + save::SaveDirectory, secondary::{ CompositeData, LAMBDA_BIDEGREE, SecondaryHomotopy, SecondaryLift, SecondaryResolutionHomomorphism, @@ -570,12 +564,6 @@ pub(crate) mod secondary { ); } - if let Some(p) = underlying.save_dir().write() { - for subdir in SaveKind::secondary_data() { - subdir.create_dir(p).unwrap(); - } - } - Self { left, right, diff --git a/ext/src/chain_complex/mod.rs b/ext/src/chain_complex/mod.rs index cf9b275801..958c5c5248 100644 --- a/ext/src/chain_complex/mod.rs +++ b/ext/src/chain_complex/mod.rs @@ -262,20 +262,6 @@ pub trait ChainComplex: Send + Sync { fn save_dir(&self) -> &SaveDirectory { &SaveDirectory::None } - - /// Get the save file of a bidegree - fn save_file( - &self, - kind: crate::save::SaveKind, - b: Bidegree, - ) -> crate::save::SaveFile { - crate::save::SaveFile { - algebra: self.algebra(), - kind, - b, - idx: None, - } - } } /// An iterator returned by [`ChainComplex::iter_stem`] diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index 8d1919136a..95e2ea0784 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -14,7 +14,6 @@ use std::{ fmt::Display, - io, sync::{Arc, Mutex, mpsc}, }; @@ -30,7 +29,7 @@ use anyhow::anyhow; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use fp::{ matrix::{AugmentedMatrix, Matrix}, - prime::{TWO, ValidPrime}, + prime::{Prime, TWO, ValidPrime}, vector::{FpSlice, FpSliceMut, FpVector}, }; use itertools::Itertools; @@ -39,8 +38,8 @@ use sseq::coordinates::{Bidegree, BidegreeGenerator}; use crate::{ chain_complex::{AugmentedChainComplex, ChainComplex, FiniteChainComplex, FreeChainComplex}, - save::{SaveDirectory, SaveKind}, - utils::{LogWriter, parallel::ParallelGuard}, + save::{NassauCommand, NassauQiWriter, SaveDirectory, SaveKind}, + utils::parallel::ParallelGuard, }; /// See [`resolution::SenderData`](../resolution/struct.SenderData.html). This differs by not having the `new` field. @@ -205,81 +204,6 @@ impl MilnorSubalgebra { .last() .unwrap_or(Self::zero_algebra()) } - - fn to_bytes(&self, buffer: &mut impl io::Write) -> io::Result<()> { - buffer.write_u64::(self.profile.len() as u64)?; - buffer.write_all(&self.profile)?; - - let len = self.profile.len(); - let zeros = [0; 8]; - let padding = len - ((len / 8) * 8); - buffer.write_all(&zeros[0..padding]) - } - - fn from_bytes(data: &mut impl io::Read) -> io::Result { - let len = data.read_u64::()? as usize; - let mut profile = vec![0; len]; - - data.read_exact(&mut profile)?; - - let padding = len - ((len / 8) * 8); - if padding > 0 { - let mut buf: [u8; 8] = [0; 8]; - data.read_exact(&mut buf[0..padding])?; - assert_eq!(buf, [0; 8]); - } - Ok(Self { profile }) - } - - fn signature_to_bytes(signature: &[PPartEntry], buffer: &mut impl io::Write) -> io::Result<()> { - if cfg!(target_endian = "little") && std::mem::size_of::() == 2 { - unsafe { - let buf: &[u8] = std::slice::from_raw_parts( - signature.as_ptr() as *const u8, - signature.len() * 2, - ); - buffer.write_all(buf).unwrap(); - } - } else { - for &entry in signature { - buffer.write_u16::(entry as u16)?; - } - } - - let len = signature.len(); - let zeros = [0; 8]; - let padding = len - ((len / 4) * 4); - - if padding > 0 { - buffer.write_all(&zeros[0..padding * 2])?; - } - Ok(()) - } - - fn signature_from_bytes(&self, data: &mut impl io::Read) -> io::Result> { - let len = self.profile.len(); - let mut signature: Vec = vec![0; len]; - - if cfg!(target_endian = "little") && std::mem::size_of::() == 2 { - unsafe { - let buf: &mut [u8] = - std::slice::from_raw_parts_mut(signature.as_mut_ptr() as *mut u8, len * 2); - data.read_exact(buf).unwrap(); - } - } else { - for entry in &mut signature { - *entry = data.read_u16::()? as PPartEntry; - } - } - - let padding = len - ((len / 4) * 4); - if padding > 0 { - let mut buffer: [u8; 8] = [0; 8]; - data.read_exact(&mut buffer[0..padding * 2])?; - assert_eq!(buffer, [0; 8]); - } - Ok(signature) - } } impl Display for MilnorSubalgebra { @@ -381,13 +305,6 @@ impl Iterator for SignatureIterator<'_> { } } -/// Some magic constants used in the save file -enum Magic { - End = -1, - Signature = -2, - Fix = -3, -} - /// A resolution of `S_2` using Nassau's algorithm. /// /// This aims to have an API similar to that of @@ -428,11 +345,9 @@ impl> Resolution { .max_degree() .ok_or_else(|| anyhow!("Nassau's algorithm requires bounded module"))?; let target = Arc::new(FiniteChainComplex::ccdz(module)); - - if let Some(p) = save_dir.write() { - for subdir in SaveKind::nassau_data() { - subdir.create_dir(p)?; - } + if let Some(store) = save_dir.store() { + let algebra = target.algebra(); + store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; } Ok(Self { @@ -495,56 +410,54 @@ impl> Resolution { }); } - #[tracing::instrument(skip_all, fields(throughput))] + #[tracing::instrument(skip_all)] fn write_qi( - f: &mut Option, + w: &mut Option, scratch: &mut FpVector, signature: &[PPartEntry], next_mask: &[usize], full_matrix: &Matrix, masked_matrix: &AugmentedMatrix<2>, - ) -> io::Result<()> { - let f = match f { - Some(f) => f, + ) -> anyhow::Result<()> { + let w = match w { + Some(w) => w, None => return Ok(()), }; - let mut own_f = LogWriter::new(f); - let f = &mut own_f; - let pivots = &masked_matrix.pivots()[0..masked_matrix.end[0]]; if !pivots.iter().any(|&x| x >= 0) { return Ok(()); } - // Write signature if non-zero. + // Emit a signature command if non-zero. if signature.iter().any(|&x| x > 0) { - f.write_u64::(Magic::Signature as u64)?; - MilnorSubalgebra::signature_to_bytes(signature, f)?; + let sig_u16: Vec = signature.iter().map(|&x| x as u16).collect(); + w.write_signature(&sig_u16)?; } - // Write quasi-inverses + // Emit one pivot command per non-trivial pivot row. for (col, &row) in pivots.iter().enumerate() { if row < 0 { continue; } - f.write_u64::(next_mask[col] as u64)?; let preimage = masked_matrix.row_segment(row as usize, 1, 1); scratch.set_scratch_vector_size(preimage.len()); scratch.as_slice_mut().assign(preimage); - scratch.to_bytes(f)?; + // The lift slice we want to write is `scratch.as_slice()` here; + // we have to capture it before reusing `scratch` for the image. + let lift_vec = scratch.clone(); scratch.set_scratch_vector_size(full_matrix.columns()); for (i, _) in preimage.iter_nonzero() { scratch.as_slice_mut().add(full_matrix.row(i), 1); } - scratch.to_bytes(f)?; + w.write_pivot( + next_mask[col] as u64, + lift_vec.as_slice(), + scratch.as_slice(), + )?; } - tracing::Span::current().record( - "throughput", - tracing::field::display(own_f.into_throughput()), - ); Ok(()) } @@ -554,16 +467,17 @@ impl> Resolution { num_new_gens: usize, target_dim: usize, ) -> anyhow::Result<()> { - if let Some(dir) = self.save_dir.write() { - let mut f = self - .save_file(SaveKind::NassauDifferential, b) - .create_file(dir.clone(), false); - f.write_u64::(num_new_gens as u64)?; - f.write_u64::(target_dim as u64)?; + if let Some(store) = self.save_dir.store() { + let mut buf = Vec::new(); + buf.write_u64::(num_new_gens as u64)?; + buf.write_u64::(target_dim as u64)?; for n in 0..num_new_gens { - self.differential(b.s()).output(b.t(), n).to_bytes(&mut f)?; + self.differential(b.s()) + .output(b.t(), n) + .to_bytes(&mut buf)?; } + store.write(SaveKind::NassauDifferential, b, &buf)?; } Ok(()) } @@ -598,14 +512,14 @@ impl> Resolution { let next = &self.modules[b.s() - 2]; next.compute_basis(b.t()); - let mut f = if let Some(dir) = self.save_dir().write() { - let mut f = self - .save_file(SaveKind::NassauQi, b - Bidegree::s_t(1, 0)) - .create_file(dir.to_owned(), true); - f.write_u64::(next.dimension(b.t()) as u64)?; - f.write_u64::(target_masked_dim as u64)?; - subalgebra.to_bytes(&mut f)?; - Some(f) + let mut f = if let Some(store) = self.save_dir.store() { + let qi_b = b - Bidegree::s_t(1, 0); + Some(store.nassau_qi_writer( + qi_b, + next.dimension(b.t()) as u64, + target_masked_dim as u64, + &subalgebra.profile, + )?) } else { None }; @@ -642,7 +556,7 @@ impl> Resolution { if let Some(f) = &mut f && target.max_computed_degree() < b.t() { - f.write_u64::(Magic::Fix as u64)?; + f.write_fix()?; } // Compute image @@ -736,8 +650,8 @@ impl> Resolution { end(); - if let Some(f) = &mut f { - f.write_u64::(Magic::End as u64)?; + if let Some(w) = f { + w.finish()?; } self.write_differential(b, num_new_gens, target_dim)?; @@ -874,13 +788,12 @@ impl> Resolution { return Ok(()); } - if let Some(dir) = self.save_dir.read() - && let Some(mut f) = self - .save_file(SaveKind::NassauDifferential, b) - .open_file(dir.clone()) + if let Some(store) = self.save_dir.store() + && let Some(data) = store.read(SaveKind::NassauDifferential, b)? { tracing::info!(%b, "Loading differential"); + let mut f = std::io::Cursor::new(data); let num_new_gens = f.read_u64::()? as usize; // This need not be equal to `target_res_dimension`. If we saved a big resolution // and now only want to load up to a small stem, then `target_res_dimension` will @@ -1057,21 +970,39 @@ impl> ChainComplex for Resolution { for<'a> &'a mut T: Into>, for<'a> &'a S: Into>, { - let mut f = if let Some(dir) = self.save_dir.read() { - if let Some(f) = self.save_file(SaveKind::NassauQi, b).open_file(dir.clone()) { - f - } else { - return false; - } - } else { - return false; + self.apply_quasi_inverse_fallible(results, b, inputs) + .unwrap_or_else(|e| panic!("apply_quasi_inverse failed at {b}: {e:#}")) + } +} + +impl> Resolution { + /// The fallible core of [`ChainComplex::apply_quasi_inverse`]. + /// + /// Returns `Ok(false)` when no quasi-inverse is available (no save store or no saved qi for + /// this bidegree), `Ok(true)` when the qi was successfully applied, and propagates + /// `anyhow::Error` from the structured zarr reader or `FpVector::update_from_bytes`. + fn apply_quasi_inverse_fallible( + &self, + results: &mut [T], + b: Bidegree, + inputs: &[S], + ) -> anyhow::Result + where + for<'a> &'a mut T: Into>, + for<'a> &'a S: Into>, + { + let Some(store) = self.save_dir.store() else { + return Ok(false); + }; + let Some(reader) = store.nassau_qi_reader(b)? else { + return Ok(false); }; let p = self.prime(); - let target_dim = f.read_u64::().unwrap() as usize; - let zero_mask_dim = f.read_u64::().unwrap() as usize; - let subalgebra = MilnorSubalgebra::from_bytes(&mut f).unwrap(); + let target_dim = reader.target_dim() as usize; + let zero_mask_dim = reader.zero_mask_dim() as usize; + let subalgebra = MilnorSubalgebra::new(reader.subalgebra_profile().to_vec()); let source = &self.modules[b.s()]; let target = &self.modules[b.s() - 1]; let algebra = target.algebra(); @@ -1088,24 +1019,24 @@ impl> ChainComplex for Resolution { let mut scratch0 = FpVector::new(p, zero_mask_dim); let mut scratch1 = FpVector::new(p, target_dim); - // If the quasi-inverse was computed using incomplete information, we need to figure out - // what the differentials in this bidegree hit and use them to lift. these variables are - // trivial if there is no such problem. + // If the quasi-inverse was computed using incomplete information, we need to figure + // out what the differentials in this bidegree hit and use them to lift. these + // variables are trivial if there is no such problem. // // target_zero_mask is the signature mask of the target under the zero signature. // // dx_matrix is an AugmentedMatrix::<3>. // - // Each row of this matrix is of the form [r; dx; x], where x is an element of the source - // of signature zero, expressed in the masked basis, and dx is the value of the - // differential on x. Then r is the entries of dx that have zero signature, which we - // include so that the rref of the matix is nice. In practice, we keep r empty until the - // very end, and then populate it manually. + // Each row of this matrix is of the form [r; dx; x], where x is an element of the + // source of signature zero, expressed in the masked basis, and dx is the value of + // the differential on x. Then r is the entries of dx that have zero signature, + // which we include so that the rref of the matix is nice. In practice, we keep r + // empty until the very end, and then populate it manually. // - // At the beginning the x's will be the new generators in this bidegree. As we read in the - // quasi-inverses for the zero signature, we keep on reducing this so that dx is zero in - // the pivot columns of the quasi-inverse. We can then use (the rref of) this matrix to - // lift remaining elements with zero signature. + // At the beginning the x's will be the new generators in this bidegree. As we read + // in the quasi-inverses for the zero signature, we keep on reducing this so that dx + // is zero in the pivot columns of the quasi-inverse. We can then use (the rref of) + // this matrix to lift remaining elements with zero signature. let (mut target_zero_mask, mut dx_matrix) = if zero_mask_dim != mask.len() { let num_new_gens = source.number_of_gens_in_degree(b.t()); assert_eq!(mask.len(), zero_mask_dim + num_new_gens); @@ -1135,83 +1066,90 @@ impl> ChainComplex for Resolution { (Vec::new(), AugmentedMatrix::<3>::new(p, 0, [0, 0, 0])) }; - loop { - let col = f.read_u64::().unwrap() as usize; - if col == Magic::End as usize { - break; - } else if col == Magic::Signature as usize { - let signature = subalgebra.signature_from_bytes(&mut f).unwrap(); - - mask.clear(); - mask.extend(subalgebra.signature_mask(&algebra, source, b.t(), &signature)); - scratch0.set_scratch_vector_size(mask.len()); - } else if col == Magic::Fix as usize { - // We need to fix the differential problem - // - // First manually add_masked the second segment to the first, which we use for - // row reduction. We do this manually for borrow checker reasons. - for (j, &k) in target_zero_mask.iter().enumerate() { - for i in 0..dx_matrix.rows() { - if dx_matrix.row_segment(i, 1, 1).entry(k) != 0 { - dx_matrix.row_segment_mut(i, 0, 0).add_basis_element(j, 1); + for cmd in reader { + match cmd? { + NassauCommand::Signature(sig_u16) => { + let signature: Vec = + sig_u16.iter().map(|&x| x as PPartEntry).collect(); + mask.clear(); + mask.extend(subalgebra.signature_mask(&algebra, source, b.t(), &signature)); + scratch0.set_scratch_vector_size(mask.len()); + } + NassauCommand::Fix => { + // We need to fix the differential problem + // + // First manually add_masked the second segment to the first, which we + // use for row reduction. We do this manually for borrow checker reasons. + for (j, &k) in target_zero_mask.iter().enumerate() { + for i in 0..dx_matrix.rows() { + if dx_matrix.row_segment(i, 1, 1).entry(k) != 0 { + dx_matrix.row_segment_mut(i, 0, 0).add_basis_element(j, 1); + } } } - } - dx_matrix.row_reduce(); + dx_matrix.row_reduce(); - // Now reduce by these elements - for i in 0..dx_matrix.rows() { - let masked_col = dx_matrix.row(i).first_nonzero().unwrap().0; - assert_eq!(dx_matrix.pivots()[masked_col], i as isize); - let col = target_zero_mask[masked_col]; + // Now reduce by these elements + for i in 0..dx_matrix.rows() { + let masked_col = dx_matrix.row(i).first_nonzero().unwrap().0; + assert_eq!(dx_matrix.pivots()[masked_col], i as isize); + let col = target_zero_mask[masked_col]; + + for (input, output) in inputs.iter_mut().zip(results.iter_mut()) { + let entry = input.entry(col); + if entry != 0 { + output.into().add_unmasked( + dx_matrix.row_segment(i, 2, 2), + 1, + &mask, + ); + input.as_slice_mut().add(dx_matrix.row_segment(i, 1, 1), 1); + } + } + } + // Drop these objects to save a bit of memory + target_zero_mask = Vec::new(); + dx_matrix = AugmentedMatrix::<3>::new(p, 0, [0, 0, 0]); + } + NassauCommand::Pivot { + col, + lift_bytes, + image_bytes, + } => { + let col = col as usize; + scratch0.update_from_bytes(&mut &lift_bytes[..])?; + scratch1.update_from_bytes(&mut &image_bytes[..])?; for (input, output) in inputs.iter_mut().zip(results.iter_mut()) { let entry = input.entry(col); if entry != 0 { - output - .into() - .add_unmasked(dx_matrix.row_segment(i, 2, 2), 1, &mask); - input.as_slice_mut().add(dx_matrix.row_segment(i, 1, 1), 1); + output.into().add_unmasked(scratch0.as_slice(), 1, &mask); + // If we resume a resolve_through_stem, input may be longer + // than scratch1. + input + .slice_mut(0, scratch1.len()) + .add(scratch1.as_slice(), 1); } } - } - - // Drop these objects to save a bit of memory - target_zero_mask = Vec::new(); - dx_matrix = AugmentedMatrix::<3>::new(p, 0, [0, 0, 0]); - } else { - scratch0.update_from_bytes(&mut f).unwrap(); - scratch1.update_from_bytes(&mut f).unwrap(); - for (input, output) in inputs.iter_mut().zip(results.iter_mut()) { - let entry = input.entry(col); - if entry != 0 { - output.into().add_unmasked(scratch0.as_slice(), 1, &mask); - // If we resume a resolve_through_stem, input may be longer than scratch1. - input - .slice_mut(0, scratch1.len()) - .add(scratch1.as_slice(), 1); - } - } - // Row reduce the differentials - if !target_zero_mask.is_empty() { - for i in 0..dx_matrix.rows() { - if dx_matrix.row_segment(i, 1, 1).entry(col) != 0 { - dx_matrix - .row_segment_mut(i, 2, 2) - .slice_mut(0, zero_mask_dim) - .add(scratch0.as_slice(), 1); - dx_matrix - .row_segment_mut(i, 1, 1) - .slice_mut(0, target_dim) - .add(scratch1.as_slice(), 1); + // Row reduce the differentials + if !target_zero_mask.is_empty() { + for i in 0..dx_matrix.rows() { + if dx_matrix.row_segment(i, 1, 1).entry(col) != 0 { + dx_matrix + .row_segment_mut(i, 2, 2) + .slice_mut(0, zero_mask_dim) + .add(scratch0.as_slice(), 1); + dx_matrix + .row_segment_mut(i, 1, 1) + .slice_mut(0, target_dim) + .add(scratch1.as_slice(), 1); + } } } } } } - // Make sure we have finished reading everything - drop(f); for dx in inputs { assert!( @@ -1220,7 +1158,7 @@ impl> ChainComplex for Resolution { target.element_to_string(b.t(), dx.as_slice()) ); } - true + Ok(true) } } diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index d37edd20ed..6e5103b529 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -10,14 +10,15 @@ use algebra::{ }, }; use anyhow::Context; -use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use dashmap::DashMap; use fp::{ matrix::{AugmentedMatrix, QuasiInverse, Subspace}, + prime::Prime, vector::{FpSlice, FpSliceMut, FpVector}, }; use itertools::Itertools; use once::{OnceBiVec, OnceVec}; +use serde::{Deserialize, Serialize}; use sseq::coordinates::{Bidegree, BidegreeGenerator}; use crate::{ @@ -26,6 +27,15 @@ use crate::{ utils::parallel::ParallelGuard, }; +#[derive(Serialize, Deserialize)] +struct DifferentialPayload { + num_gens: usize, + target_res_dimension: usize, + target_cc_dimension: usize, + differentials: Vec, + chain_maps: Vec, +} + /// In [`MuResolution::compute_through_stem`] and [`MuResolution::compute_through_bidegree`], we pass /// this struct around to inform the supervisor what bidegrees have been computed. We use an /// explicit struct instead of a tuple to avoid an infinite type problem. @@ -127,15 +137,12 @@ where ) -> anyhow::Result { let save_dir = save_dir.into(); let algebra = complex.algebra(); + if let Some(store) = save_dir.store() { + store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; + } let min_degree = complex.min_degree(); let zero_module = Arc::new(MuFreeModule::new(algebra, "F_{-1}".to_string(), min_degree)); - if let Some(p) = save_dir.write() { - for subdir in SaveKind::resolution_data() { - subdir.create_dir(p)?; - } - } - Ok(Self { name: String::new(), complex, @@ -222,11 +229,14 @@ where let p = self.prime(); - if let Some(dir) = self.save_dir.read() - && let Some(mut f) = self.save_file(SaveKind::Kernel, b).open_file(dir.clone()) - { - return Subspace::from_bytes(p, &mut f) + if let Some(store) = self.save_dir.store() + && let Some(data) = store + .read(SaveKind::Kernel, b) .with_context(|| format!("Failed to read kernel at {b}")) + .unwrap() + { + return bitcode::deserialize::(&data) + .with_context(|| format!("Failed to deserialize kernel at {b}")) .unwrap(); } @@ -264,13 +274,11 @@ where let kernel = matrix.compute_kernel(); if self.should_save - && let Some(dir) = self.save_dir.write() + && let Some(store) = self.save_dir.store() { - let mut f = self - .save_file(SaveKind::Kernel, b) - .create_file(dir.clone(), true); - kernel - .to_bytes(&mut f) + let bytes = bitcode::serialize(&kernel).unwrap(); + store + .write(SaveKind::Kernel, b, &bytes) .with_context(|| format!("Failed to write kernel at {b}")) .unwrap(); } @@ -396,43 +404,38 @@ where target_res.compute_basis(b.t()); let target_res_dimension = target_res.dimension(b.t()); - if let Some(dir) = self.save_dir.read() - && let Some(mut f) = self - .save_file(SaveKind::Differential, b) - .open_file(dir.clone()) + if let Some(store) = self.save_dir.store() + && let Some(data) = store + .read(SaveKind::Differential, b) + .with_context(|| format!("Failed to read differential at {b}")) + .unwrap() { - let num_new_gens = f.read_u64::().unwrap() as usize; - // This need not be equal to `target_res_dimension`. If we saved a big resolution - // and now only want to load up to a small stem, then `target_res_dimension` will - // be smaller. If we have previously saved a small resolution up to a stem and now - // want to resolve further, it will be bigger. - let saved_target_res_dimension = f.read_u64::().unwrap() as usize; + let payload: DifferentialPayload = bitcode::deserialize(&data) + .with_context(|| format!("Failed to deserialize differential at {b}")) + .unwrap(); + + let num_new_gens = payload.num_gens; assert_eq!( - target_cc_dimension, - f.read_u64::().unwrap() as usize, + target_cc_dimension, payload.target_cc_dimension, "Malformed data: mismatched augmentation target dimension" ); self.add_generators(b, num_new_gens); - let mut d_targets = Vec::with_capacity(num_new_gens); - let mut a_targets = Vec::with_capacity(num_new_gens); - - for _ in 0..num_new_gens { - d_targets - .push(FpVector::from_bytes(p, saved_target_res_dimension, &mut f).unwrap()); - } - for _ in 0..num_new_gens { - a_targets.push(FpVector::from_bytes(p, target_cc_dimension, &mut f).unwrap()); - } - drop(f); - current_differential.add_generators_from_rows(b.t(), d_targets); - current_chain_map.add_generators_from_rows(b.t(), a_targets); + current_differential.add_generators_from_rows(b.t(), payload.differentials); + current_chain_map.add_generators_from_rows(b.t(), payload.chain_maps); - // res qi + // res qi — structured zarr layout (pivots + chunked rows) if self.load_quasi_inverse { - if let Some(mut f) = self.save_file(SaveKind::ResQi, b).open_file(dir.clone()) { - let res_qi = QuasiInverse::from_bytes(p, &mut f).unwrap(); + if let Some(reader) = store + .stream_res_qi(b, self.prime()) + .with_context(|| format!("Failed to read res qi at {b}")) + .unwrap() + { + let res_qi = reader + .into_quasi_inverse() + .with_context(|| format!("Failed to deserialize res qi at {b}")) + .unwrap(); assert_eq!( res_qi.source_dimension(), @@ -448,11 +451,14 @@ where current_differential.set_quasi_inverse(b.t(), None); } - if let Some(mut f) = self - .save_file(SaveKind::AugmentationQi, b) - .open_file(dir.clone()) + if let Some(qi_data) = store + .read(SaveKind::AugmentationQi, b) + .with_context(|| format!("Failed to read augmentation qi at {b}")) + .unwrap() { - let cm_qi = QuasiInverse::from_bytes(p, &mut f).unwrap(); + let cm_qi: QuasiInverse = bitcode::deserialize(&qi_data) + .with_context(|| format!("Failed to deserialize augmentation qi at {b}")) + .unwrap(); assert_eq!( cm_qi.target_dimension(), @@ -499,14 +505,11 @@ where if !self.has_computed_bidegree(b + Bidegree::s_t(1, 0)) { let kernel = matrix.compute_kernel(); if self.should_save - && let Some(dir) = self.save_dir.write() + && let Some(store) = self.save_dir.store() { - let mut f = self - .save_file(SaveKind::Kernel, b) - .create_file(dir.clone(), true); - - kernel - .to_bytes(&mut f) + let bytes = bitcode::serialize(&kernel).unwrap(); + store + .write(SaveKind::Kernel, b, &bytes) .with_context(|| format!("Failed to write kernel at {b}")) .unwrap(); } @@ -658,58 +661,47 @@ where ); if self.should_save - && let Some(dir) = self.save_dir.write() + && let Some(store) = self.save_dir.store() { - // Write differentials last, because if we were terminated halfway, we want the - // differentials to exist iff everything has been written. However, we start by - // opening the differentials first to make sure we are not overwriting anything. - - // Open differentials file - let mut f = self - .save_file(SaveKind::Differential, b) - .create_file(dir.clone(), false); - - // Write resolution qi - res_qi - .to_bytes( - &mut self - .save_file(SaveKind::ResQi, b) - .create_file(dir.clone(), true), - ) + // Write resolution qi as a structured group (pivots + rows arrays). + store + .write_res_qi(b, &res_qi) + .with_context(|| format!("Failed to write res qi at {b}")) .unwrap(); // Write augmentation qi - cm_qi - .to_bytes( - &mut self - .save_file(SaveKind::AugmentationQi, b) - .create_file(dir.clone(), true), - ) + let qi_bytes = bitcode::serialize(&cm_qi).unwrap(); + store + .write(SaveKind::AugmentationQi, b, &qi_bytes) + .with_context(|| format!("Failed to write augmentation qi at {b}")) .unwrap(); - // Write differentials - f.write_u64::(num_new_gens as u64).unwrap(); - f.write_u64::(target_res_dimension as u64) - .unwrap(); - f.write_u64::(target_cc_dimension as u64) + // Build and write differential payload last for crash safety + let differentials = (0..num_new_gens) + .map(|n| current_differential.output(b.t(), n).clone()) + .collect(); + let chain_maps = (0..num_new_gens) + .map(|n| current_chain_map.output(b.t(), n).clone()) + .collect(); + let payload = DifferentialPayload { + num_gens: num_new_gens, + target_res_dimension, + target_cc_dimension, + differentials, + chain_maps, + }; + let payload_bytes = bitcode::serialize(&payload).unwrap(); + store + .write(SaveKind::Differential, b, &payload_bytes) + .with_context(|| format!("Failed to write differential at {b}")) .unwrap(); - for n in 0..num_new_gens { - current_differential - .output(b.t(), n) - .to_bytes(&mut f) - .unwrap(); - } - for n in 0..num_new_gens { - current_chain_map.output(b.t(), n).to_bytes(&mut f).unwrap(); - } - drop(f); - // Delete kernel if b.s() > 0 { - self.save_file(SaveKind::Kernel, b - Bidegree::s_t(1, 0)) - .delete_file(dir.clone()) - .unwrap(); + let prev = Bidegree::n_s(b.n() + 1, b.s() - 1); + store.delete(SaveKind::Kernel, prev).unwrap_or_else(|e| { + tracing::warn!("Failed to delete kernel at ({}, {}): {e}", b.s() - 1, b.t()); + }); } } @@ -870,11 +862,13 @@ where } else if distance == 1 && b.s() < max.s() { // We compute the kernel at the edge if necessary let next_b = b + Bidegree::s_t(0, 1); - if !self.has_computed_bidegree(b + Bidegree::s_t(1, 1)) + let check_b = b + Bidegree::s_t(1, 1); + if !self.has_computed_bidegree(check_b) && (self.save_dir.is_none() || !self - .save_file(SaveKind::Differential, b + Bidegree::s_t(1, 1)) - .exists(self.save_dir.read().cloned().unwrap())) + .save_dir + .store() + .is_some_and(|store| store.exists(SaveKind::Differential, check_b))) { scope.spawn(move |_| { self.kernels.insert(next_b, self.get_kernel(next_b)); @@ -945,9 +939,9 @@ where qi.apply(result.into(), 1, input.into()); } true - } else if let Some(dir) = self.save_dir.read() { - if let Some(mut f) = self.save_file(SaveKind::ResQi, b).open_file(dir.clone()) { - QuasiInverse::stream_quasi_inverse(self.prime(), &mut f, results, inputs).unwrap(); + } else if let Some(store) = self.save_dir.store() { + if let Some(reader) = store.stream_res_qi(b, self.prime()).unwrap() { + reader.apply(results, inputs).unwrap(); true } else { false @@ -993,7 +987,7 @@ pub(crate) mod secondary { use crate::{ chain_complex::FreeChainComplex, - save::{SaveDirectory, SaveKind}, + save::SaveDirectory, secondary::{CompositeData, SecondaryHomotopy, SecondaryLift}, }; @@ -1088,12 +1082,6 @@ pub(crate) mod secondary { CC::Algebra: PairAlgebra, { pub fn new(cc: Arc) -> Self { - if let Some(p) = cc.save_dir().write() { - for subdir in SaveKind::secondary_data() { - subdir.create_dir(p).unwrap(); - } - } - Self { underlying: cc, homotopies: OnceBiVec::new(2), diff --git a/ext/src/resolution_homomorphism.rs b/ext/src/resolution_homomorphism.rs index 271c7318d3..d3d3380ae5 100644 --- a/ext/src/resolution_homomorphism.rs +++ b/ext/src/resolution_homomorphism.rs @@ -9,7 +9,6 @@ use algebra::{ homomorphism::{ModuleHomomorphism, MuFreeModuleHomomorphism}, }, }; -use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use fp::{ matrix::Matrix, vector::{FpSliceMut, FpVector}, @@ -39,6 +38,10 @@ where pub target: Arc, maps: OnceBiVec>>, pub shift: Bidegree, + /// Save directory for this homomorphism's chain map and the data of any + /// secondary lift built from it. Set to a `products/{name}` subgroup of + /// the source's save_dir when `name` is non-empty; `None` otherwise so + /// that anonymous homomorphisms don't pollute the store. save_dir: SaveDirectory, } @@ -49,17 +52,17 @@ where CC2: ChainComplex, { pub fn new(name: String, source: Arc, target: Arc, shift: Bidegree) -> Self { - let save_dir = if source.save_dir().is_some() && !name.is_empty() { - let mut save_dir = source.save_dir().clone(); - save_dir.push(format!("products/{name}")); - SaveKind::ChainMap - .create_dir(save_dir.write().unwrap()) - .unwrap(); - save_dir + let save_dir = if !name.is_empty() + && let Some(parent) = source.save_dir().store() + { + SaveDirectory::Store( + parent + .subgroup(&format!("products/{name}")) + .expect("Failed to create products subgroup"), + ) } else { SaveDirectory::None }; - Self { name, source, @@ -231,35 +234,27 @@ where ); } - if let Some(dir) = self.save_dir.read() { + if let Some(store) = self.save_dir.store() + && let Some(data) = store.read(SaveKind::ChainMap, input).unwrap() + { + let mut cursor = &data[..]; let mut outputs = Vec::with_capacity(num_gens); - - if let Some(mut f) = self - .source - .save_file(SaveKind::ChainMap, input) - .open_file(dir.to_owned()) - { - let fx_dimension = f.read_u64::().unwrap() as usize; - for _ in 0..num_gens { - outputs.push(FpVector::from_bytes(p, fx_dimension, &mut f).unwrap()); - } - return f_cur.add_generators_from_rows_ooo(input.t(), outputs); + for _ in 0..num_gens { + outputs.push(FpVector::from_bytes(p, fx_dimension, &mut cursor).unwrap()); } + return f_cur.add_generators_from_rows_ooo(input.t(), outputs); } if output.s() == 0 { let outputs = extra_images.unwrap_or_else(|| vec![FpVector::new(p, fx_dimension); num_gens]); - if let Some(dir) = self.save_dir.write() { - let mut f = self - .source - .save_file(SaveKind::ChainMap, input) - .create_file(dir.clone(), false); - f.write_u64::(fx_dimension as u64).unwrap(); + if let Some(store) = self.save_dir.store() { + let mut buf = Vec::new(); for row in &outputs { - row.to_bytes(&mut f).unwrap(); + row.to_bytes(&mut buf).unwrap(); } + store.write(SaveKind::ChainMap, input, &buf).unwrap(); } return f_cur.add_generators_from_rows_ooo(input.t(), outputs); @@ -327,15 +322,12 @@ where ); } - if let Some(dir) = self.save_dir.write() { - let mut f = self - .source - .save_file(SaveKind::ChainMap, input) - .create_file(dir.clone(), false); - f.write_u64::(fx_dimension as u64).unwrap(); + if let Some(store) = self.save_dir.store() { + let mut buf = Vec::new(); for row in &outputs { - row.to_bytes(&mut f).unwrap(); + row.to_bytes(&mut buf).unwrap(); } + store.write(SaveKind::ChainMap, input, &buf).unwrap(); } f_cur.add_generators_from_rows_ooo(input.t(), outputs) } @@ -516,7 +508,7 @@ pub(crate) mod secondary { use super::ResolutionHomomorphism; use crate::{ chain_complex::FreeChainComplex, - save::{SaveDirectory, SaveKind}, + save::SaveDirectory, secondary::{ CompositeData, LAMBDA_BIDEGREE, SecondaryHomotopy, SecondaryLift, SecondaryResolution, }, @@ -677,12 +669,6 @@ pub(crate) mod secondary { assert!(Arc::ptr_eq(&underlying.source, &source.underlying())); assert!(Arc::ptr_eq(&underlying.target, &target.underlying())); - if let Some(p) = underlying.save_dir().write() { - for subdir in SaveKind::secondary_data() { - subdir.create_dir(p).unwrap(); - } - } - Self { source, target, diff --git a/ext/src/save.rs b/ext/src/save.rs index 536f0b3d23..8d635249d0 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -1,507 +1,1300 @@ +//! Two-tier zarr save system for resolutions, chain maps, and chain homotopies. +//! +//! On native targets the store is backed by a real zarr v3 store on the local filesystem via +//! [`zarrs::filesystem::FilesystemStore`]. On `wasm32-unknown-unknown` we swap in +//! [`zarrs::storage::store::MemoryStore`] instead — `zarrs_filesystem` transitively pulls in +//! `positioned-io::RandomAccessFile`, which is gated to `cfg(any(windows, unix))` and doesn't +//! compile for WASM. The WASM frontend has no filesystem to persist to anyway, so the memory +//! store just acts as a no-op sink that's dropped at session end; the same code paths +//! exercise it on both targets. +//! +//! # Layout +//! +//! One zarr v3 store with per-namespace subgroups for named homomorphisms and chain homotopies: +//! +//! ```text +//! save_dir.zarr/ +//! zarr.json # root group +//! +//! # Main resolution — shard + stream tier at root +//! {kind}/zarr.json + c/... # 2D or 3D vlen-bytes shard +//! qi/n{n}_s{s}/{kind}/ # per-bidegree group, kind-specific sub-arrays +//! +//! # Named ResolutionHomomorphism — one subgroup per name +//! products/{name}/ +//! chain_map/, secondary_composite/, secondary_intermediate/, secondary_homotopy/ +//! +//! # Named ChainHomotopy — one subgroup per (left_name, right_name) +//! homotopies/{left}__{right}/ +//! chain_homotopy/, secondary_composite/, secondary_intermediate/, secondary_homotopy/ +//! ``` +//! +//! # Two tiers +//! +//! **Shard tier.** Small payloads (differentials, kernels, chain maps, secondary data) use a +//! `vlen-bytes` sharded array per kind, with shard shape `[SHARD_N, SHARD_S(, SHARD_IDX)]`, +//! inner chunk shape `[1, 1(, 1)]`, and CRC32C over each shard (no zstd — the payloads are too +//! small to benefit). +//! +//! **Stream tier.** Large payloads ([`SaveKind::ResQi`], [`SaveKind::NassauQi`]) use +//! per-bidegree zarr *groups* with kind-specific sub-arrays: +//! +//! - `res_qi/` — group attributes hold the scalar dimensions; `pivots/` is a 1D `i64` array, +//! `rows/` is a 2D `u8` array shaped `[image_dim, num_limbs * 8]`, chunked over rows. +//! - `nassau_qi/` — group attributes hold `target_dim`, `zero_mask_dim`, `subalgebra_profile`, +//! `num_commands`, `finished`. `commands/` is a 1D vlen-bytes array with one element per +//! [`NassauCommand`]. +//! +//! Group attributes include a `finished` flag, which is the source of truth — readers treat the +//! data as missing if the writer was dropped before calling `finish()`. +//! +//! Subgroups share the same underlying [`FilesystemStore`] via `Arc` clone; only the `group` +//! prefix differs. Shard arrays are created lazily on first write so that subgroups don't +//! populate kinds they never use. +//! +//! # Coordinate system +//! +//! Shard arrays are indexed by `(n, s)`, matching `MultiDegree<2>::coords()` and generalizing to +//! `MultiDegree` for `N > 2`. Stems can be negative (e.g. `RP^\infty_{-k}`, A-module shifts), +//! and zarr v3 has no native support for negative chunk indices, so we apply a fixed internal +//! offset: every caller-supplied `n` is shifted to `n - N_MIN` before becoming a zarr index. +//! `N_MIN` is intentionally generous (-1024) and never exposed in the public API; sparse zarr +//! arrays cost essentially nothing for the empty negative regions, so the overhead is purely in +//! `zarr.json` metadata. + use std::{ - collections::HashSet, - fs::File, - io, path::{Path, PathBuf}, - sync::{Arc, LazyLock, Mutex}, + sync::{Arc, Mutex}, }; -use algebra::Algebra; use anyhow::Context; -use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; -use sseq::coordinates::Bidegree; +use dashmap::{DashMap, DashSet}; +use fp::{ + matrix::{Matrix, QuasiInverse}, + prime::ValidPrime, + vector::{FpSlice, FpSliceMut, FpVector}, +}; +use sseq::coordinates::{Bidegree, BidegreeGenerator}; +#[cfg(not(target_arch = "wasm32"))] +use zarrs::array::codec::ZstdCodec; +// The concrete backing store depends on the target: native uses a `FilesystemStore` for real +// on-disk persistence; WASM uses an in-memory `MemoryStore` so `zarrs_filesystem` (which pulls +// in `positioned-io::RandomAccessFile`, gated to windows/unix) doesn't need to compile. The +// WASM frontend has no filesystem to persist to anyway, so the memory store just serves as a +// no-op sink and is dropped at session end. +#[cfg(not(target_arch = "wasm32"))] +use zarrs::filesystem::FilesystemStore; +#[cfg(target_arch = "wasm32")] +use zarrs::storage::store::MemoryStore; +use zarrs::{ + array::{ArrayBuilder, ArraySubset, CodecOptions, codec::Crc32cCodec, data_type}, + group::GroupBuilder, + storage::{ReadableWritableListableStorage, ReadableWritableListableStorageTraits}, +}; -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum SaveDirectory { - None, - Combined(AbsolutePath), - Split { - read: AbsolutePath, - write: AbsolutePath, - }, -} +/// Most-negative stem the on-disk layout can store. +/// +/// Hidden from callers; used internally to shift caller-supplied `n` values into the unsigned +/// zarr index space. See the module docs. +const N_MIN: i32 = -1024; -impl SaveDirectory { - pub fn read(&self) -> Option<&PathBuf> { - match self { - Self::None => None, - Self::Combined(x) => Some(&x.0), - Self::Split { read, .. } => Some(&read.0), - } - } +/// Number of slots in the n dimension. +/// +/// Effective `n` range is `[N_MIN, N_MIN + N_SPAN)` = `[-1024, 3072)` — well beyond any +/// conceivable production stem. +const N_SPAN: u64 = 4096; - pub fn write(&self) -> Option<&PathBuf> { - match self { - Self::None => None, - Self::Combined(x) => Some(&x.0), - Self::Split { write, .. } => Some(&write.0), - } - } +/// Number of slots in the s dimension. `s` is unsigned: `[0, S_SPAN)`. +const S_SPAN: u64 = 1024; - pub fn push>(&mut self, p: P) { - match self { - Self::None => {} - Self::Combined(d) => { - d.push(p); - } - Self::Split { read, write } => { - read.push(&p); - write.push(p); - } - } - } +/// Number of slots reserved for the intra-bidegree index of indexed kinds. +const IDX_SPAN: u64 = 256; - pub fn is_none(&self) -> bool { - matches!(self, Self::None) - } +/// Shard shape in the n dimension. +const SHARD_N: u64 = 8; - pub fn is_some(&self) -> bool { - !self.is_none() - } +/// Shard shape in the s dimension. +const SHARD_S: u64 = 8; + +/// Shard shape in the idx dimension. +const SHARD_IDX: u64 = 8; + +/// zstd compression level for the stream tier. +#[cfg(not(target_arch = "wasm32"))] +const ZSTD_LEVEL: i32 = 19; + +/// Convert a zarrs error into `anyhow::Error`. +/// +/// On `wasm32-unknown-unknown` some zarrs error types contain `Arc` with only +/// `MaybeSend + MaybeSync` bounds (no-ops on wasm), so `anyhow::Error`'s `Send + Sync` +/// requirement rejects them via the blanket `From` impl. Formatting the error as a string +/// sidesteps the bound and works on both targets; the tradeoff is that the original error's +/// source chain is collapsed into its `Display` output. +fn zarr_err(e: E) -> anyhow::Error { + anyhow::anyhow!("{e}") } -impl From> for SaveDirectory { - fn from(x: Option) -> Self { - match x { - None => Self::None, - Some(x) => Self::Combined(AbsolutePath::new(x)), - } +/// Build the bytes-to-bytes codec chain for stream-tier arrays. +/// +/// On native targets this is `zstd(level=19) + crc32c`. On wasm32 the `zstd` feature is +/// disabled (its C build expects POSIX symbols the wasm libc shim doesn't expose) so we fall +/// back to `crc32c` alone — the memory-backed store on wasm is ephemeral, so skipping +/// compression is fine. +fn stream_tier_codecs() -> Vec> { + #[cfg(not(target_arch = "wasm32"))] + { + vec![ + Arc::new(ZstdCodec::new(ZSTD_LEVEL, false)), + Arc::new(Crc32cCodec::new()), + ] + } + #[cfg(target_arch = "wasm32")] + { + vec![Arc::new(Crc32cCodec::new())] } } -/// A `DashSet` of files that are currently opened and being written to. When calling this -/// function for the first time, we set the ctrlc handler to delete currently opened files then -/// exit. -fn open_files() -> &'static Mutex> { - static OPEN_FILES: LazyLock>> = LazyLock::new(|| { - #[cfg(unix)] - ctrlc::set_handler(move || { - tracing::warn!("Ctrl-C detected. Deleting open files and exiting."); - let files = open_files().lock().unwrap(); - for file in &*files { - std::fs::remove_file(file) - .unwrap_or_else(|_| panic!("Error when deleting {file:?}")); - tracing::warn!(?file, "deleted"); - } - std::process::exit(130); - }) - .expect("Error setting Ctrl-C handler"); - Default::default() - }); - &OPEN_FILES -} +/// Number of matrix rows per chunk in the ResQi `rows` array. +/// +/// Bounds the memory needed to read one chunk worth of rows. +const CHUNK_RES_QI_ROWS: u64 = 1024; -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -#[non_exhaustive] -pub enum SaveKind { - /// The kernel of a resolution differential - Kernel, +/// Number of commands per chunk in the NassauQi `commands` array. +/// +/// Bounds the memory the writer buffers between chunk flushes. +const CHUNK_NASSAU_COMMANDS: u64 = 1024; - /// The differential and augmentation map in a resolution - Differential, +/// Upper bound on the number of commands in a NassauQi `commands` array. +/// +/// Used as the array shape at creation time. The actual count is committed to the `num_commands` +/// group attribute on `finish()`. +const NASSAU_QI_MAX_COMMANDS: u64 = 1 << 24; - /// The quasi-inverse of the resolution differential - ResQi, +pub struct ZarrSaveStore { + /// Filesystem root of the underlying store. Same for all subgroups. + path: PathBuf, + /// Underlying zarr storage. Cheap to clone via `Arc`. + /// + /// On `wasm32-unknown-unknown` this wraps a trait object bounded only by + /// `MaybeSend + MaybeSync` (no-ops on wasm), so the `Arc` isn't `Send`/`Sync` in Rust's + /// eyes. The rest of `ext` requires chain complexes to be `Send + Sync`, and rippling + /// that relaxation through the whole crate to accommodate the WASM frontend isn't worth + /// it. Instead we force `Send + Sync` via the `unsafe impl`s below — sound because + /// wasm32 is single-threaded, so the absent cross-thread guarantees are vacuously + /// satisfied. + store: ReadableWritableListableStorage, + /// Group prefix applied to every operation. + /// + /// Empty for the root store; e.g. `"/products/foo"` or `"/homotopies/foo__bar"` for + /// subgroups. + group: String, + /// Tracks shard-tier arrays already known to exist on disk for this `(store, group)`. + /// + /// Used to skip the `meta_path` check on subsequent writes. + created: DashSet, + /// Per-kind write lock. + /// + /// Since zarrs 0.14, `Array::store_array_subset` is documented as requiring caller-side + /// synchronization for "regions sharing any chunks" — the codec does a read-modify-write on + /// the entire shard internally, so concurrent calls touching different inner chunks of the + /// same shard race and lose writes. We serialize per kind, which is coarser than required + /// (per shard would suffice) but simpler and entirely sufficient for our workload, since + /// cross-kind parallelism dominates. + write_locks: DashMap>>, +} - /// The quasi-inverse of the augmentation map - AugmentationQi, +// See the doc comment on `ZarrSaveStore::store` for why this is sound on wasm. On native the +// trait object is already `Send + Sync`, so the cfg gate avoids a redundant-impl warning. +#[cfg(target_arch = "wasm32")] +unsafe impl Send for ZarrSaveStore {} +#[cfg(target_arch = "wasm32")] +unsafe impl Sync for ZarrSaveStore {} - /// Secondary composite - SecondaryComposite, +impl std::fmt::Debug for ZarrSaveStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ZarrSaveStore") + .field("path", &self.path) + .field("group", &self.group) + .finish_non_exhaustive() + } +} - /// Intermediate data used by secondary code - SecondaryIntermediate, +impl ZarrSaveStore { + pub fn create(path: impl AsRef) -> anyhow::Result { + let path = std::path::absolute(path.as_ref()) + .with_context(|| format!("Failed to resolve path: {:?}", path.as_ref()))?; - /// A secondary homotopy - SecondaryHomotopy, + #[cfg(not(target_arch = "wasm32"))] + let store: ReadableWritableListableStorage = Arc::new(FilesystemStore::new(&path)?); + #[cfg(target_arch = "wasm32")] + let store: ReadableWritableListableStorage = Arc::new(MemoryStore::new()); - /// A chain map - ChainMap, + // Root group. + // + // On WASM the store is in-memory, so `zarr.json` never "exists" on disk; we + // unconditionally build the root group (the memory store silently overwrites any + // existing entry). + #[cfg(not(target_arch = "wasm32"))] + let needs_root_group = !path.join("zarr.json").exists(); + #[cfg(target_arch = "wasm32")] + let needs_root_group = true; + if needs_root_group { + GroupBuilder::new() + .build(store.clone(), "/")? + .store_metadata()?; + } - /// A chain homotopy - ChainHomotopy, + Ok(Self { + path, + store, + group: String::new(), + created: DashSet::new(), + write_locks: DashMap::new(), + }) + } - /// The differential with Nassau's algorithm. This does not store the chain map data because we - /// always only resolve the sphere - NassauDifferential, + /// Bind this store to a specific algebra, catching accidental algebra / prime mismatches. + /// + /// On a fresh store (the root group's `algebra_magic` attribute is unset) this writes + /// `algebra_magic`, `prime`, and `algebra_prefix` to the root group so a later load can + /// verify them. On an already-bound store the stored magic is compared against + /// `algebra_magic` and a mismatch returns an error citing both values. + /// + /// Callers should invoke this once per resolution setup — typically from + /// `Resolution::new_with_save` — before any data is read or written. Subgroups + /// (`products/…`, `homotopies/…`) share the same underlying store and therefore the same + /// root attributes, so they inherit the check without a second call. + pub fn bind_to_algebra( + &self, + algebra_magic: u32, + prime: u32, + algebra_prefix: &str, + ) -> anyhow::Result<()> { + let root = zarrs::group::Group::open(self.store.clone(), "/").map_err(zarr_err)?; + let attrs = root.attributes(); + if let Some(stored) = attrs.get("algebra_magic").and_then(|v| v.as_u64()) { + let stored = stored as u32; + if stored != algebra_magic { + let stored_prefix = attrs + .get("algebra_prefix") + .and_then(|v| v.as_str()) + .unwrap_or("?"); + let stored_prime = attrs + .get("prime") + .and_then(|v| v.as_u64()) + .map(|p| p.to_string()) + .unwrap_or_else(|| "?".into()); + anyhow::bail!( + "Save store at {:?} was created with a different algebra: stored magic \ + {stored:#010x} ({stored_prefix} at p={stored_prime}), expected magic \ + {algebra_magic:#010x} ({algebra_prefix} at p={prime})", + self.path, + ); + } + return Ok(()); + } - /// The quasi-inverse data in Nassau's algorithm - NassauQi, -} + // Unbound store: write the magic and related fields now. We rebuild the root group's + // attributes map with the new entries merged in. + let mut new_attrs = attrs.clone(); + new_attrs.insert("algebra_magic".into(), (u64::from(algebra_magic)).into()); + new_attrs.insert("prime".into(), (u64::from(prime)).into()); + new_attrs.insert("algebra_prefix".into(), algebra_prefix.into()); + let group = GroupBuilder::new() + .attributes(new_attrs) + .build(self.store.clone(), "/") + .map_err(zarr_err)?; + group.store_metadata().map_err(zarr_err)?; + Ok(()) + } -impl SaveKind { - pub fn magic(self) -> u32 { - match self { - Self::Kernel => 0x0000D1FF, - Self::Differential => 0xD1FF0000, - Self::ResQi => 0x0100D1FF, - Self::AugmentationQi => 0x0100A000, - Self::SecondaryComposite => 0x00020000, - Self::SecondaryIntermediate => 0x00020001, - Self::SecondaryHomotopy => 0x00020002, - Self::ChainMap => 0x10100000, - Self::ChainHomotopy => 0x11110000, - Self::NassauDifferential => 0xD1FF0001, - Self::NassauQi => 0x0100D1FE, + /// Open a subgroup at `{self.group}/{name}`. + /// + /// Shares the same underlying store as `self`. The subgroup's `zarr.json` is created if + /// missing. + pub fn subgroup(&self, name: &str) -> anyhow::Result { + let group = format!("{}/{}", self.group, name); + let group_path = self.path.join(group.trim_start_matches('/')); + if !group_path.join("zarr.json").exists() { + // Ensure each path component exists as a group so that intermediate + // levels (e.g. `products/`) are valid zarr groups, not just dirs. + self.ensure_intermediate_groups(&group)?; + GroupBuilder::new() + .build(self.store.clone(), &group)? + .store_metadata()?; } + Ok(Self { + path: self.path.clone(), + store: self.store.clone(), + group, + created: DashSet::new(), + write_locks: DashMap::new(), + }) } - pub fn name(self) -> &'static str { - match self { - Self::Kernel => "kernel", - Self::Differential => "differential", - Self::ResQi => "res_qi", - Self::AugmentationQi => "augmentation_qi", - Self::SecondaryComposite => "secondary_composite", - Self::SecondaryIntermediate => "secondary_intermediate", - Self::SecondaryHomotopy => "secondary_homotopy", - Self::ChainMap => "chain_map", - Self::ChainHomotopy => "chain_homotopy", - Self::NassauDifferential => "nassau_differential", - Self::NassauQi => "nassau_qi", + /// Walk every prefix of `group` and create a zarr group for any prefix that doesn't already + /// have one (e.g. `/products/` before `/products/foo`). + fn ensure_intermediate_groups(&self, group: &str) -> anyhow::Result<()> { + // Split on '/', skipping the leading empty segment. + let segments: Vec<&str> = group.split('/').filter(|s| !s.is_empty()).collect(); + for i in 1..segments.len() { + let prefix = format!("/{}", segments[..i].join("/")); + let meta = self + .path + .join(prefix.trim_start_matches('/')) + .join("zarr.json"); + if !meta.exists() { + GroupBuilder::new() + .build(self.store.clone(), &prefix)? + .store_metadata()?; + } } + Ok(()) } - pub fn resolution_data() -> impl Iterator { - use SaveKind::*; - static KINDS: [SaveKind; 4] = [Kernel, Differential, ResQi, AugmentationQi]; - KINDS.iter().copied() + pub fn path(&self) -> &Path { + &self.path } - pub fn nassau_data() -> impl Iterator { - use SaveKind::*; - static KINDS: [SaveKind; 2] = [NassauDifferential, NassauQi]; - KINDS.iter().copied() + /// Filesystem path corresponding to this store's group prefix. + /// + /// Returns `self.path` for the root store. + fn group_fs_path(&self) -> PathBuf { + if self.group.is_empty() { + self.path.clone() + } else { + self.path.join(self.group.trim_start_matches('/')) + } } - pub fn secondary_data() -> impl Iterator { - use SaveKind::*; - static KINDS: [SaveKind; 3] = - [SecondaryComposite, SecondaryIntermediate, SecondaryHomotopy]; - KINDS.iter().copied() + fn shard_array_path(&self, kind: SaveKind) -> String { + format!("{}/{}", self.group, kind.name()) } - pub fn create_dir(self, p: &std::path::Path) -> anyhow::Result<()> { - let mut p = p.to_owned(); + /// Translate signed `(n, s, [idx])` coordinates into the unsigned zarr indices used for + /// shard arrays. + /// + /// The first coordinate (`n`) is offset by `-N_MIN`; later coordinates are passed through + /// as-is. + fn shard_zarr_coords(coords: [i32; N]) -> Vec { + let mut out = Vec::with_capacity(N); + out.push((coords[0] - N_MIN) as u64); + for &c in &coords[1..] { + out.push(c as u64); + } + out + } - p.push(format!("{}s", self.name())); - if !p.exists() { - std::fs::create_dir_all(&p) - .with_context(|| format!("Failed to create directory {p:?}"))?; - } else if !p.is_dir() { - return Err(anyhow::anyhow!("{p:?} is not a directory")); + /// Get-or-create the per-kind write lock for this store. + /// + /// See the comment on the `write_locks` field for why this exists. + fn write_lock(&self, kind: SaveKind) -> Arc> { + Arc::clone( + self.write_locks + .entry(kind) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .value(), + ) + } + + /// Lazily create the shard-tier array for `kind` under this group. + fn ensure_shard_array(&self, kind: SaveKind) -> anyhow::Result<()> { + if self.created.contains(&kind) { + return Ok(()); } + let meta_path = self.group_fs_path().join(kind.name()).join("zarr.json"); + if !meta_path.exists() { + let array_path = self.shard_array_path(kind); + let (shape, chunk_shape, subchunk) = if kind.is_indexed() { + ( + vec![N_SPAN, S_SPAN, IDX_SPAN], + vec![SHARD_N, SHARD_S, SHARD_IDX], + vec![1, 1, 1], + ) + } else { + (vec![N_SPAN, S_SPAN], vec![SHARD_N, SHARD_S], vec![1, 1]) + }; + let arr = ArrayBuilder::new( + shape, + chunk_shape, + data_type::bytes(), + zarrs::array::FillValue::from(Vec::::new()), + ) + .subchunk_shape(subchunk) + .bytes_to_bytes_codecs(vec![Arc::new(Crc32cCodec::new())]) + .build(self.store.clone(), &array_path)?; + arr.store_metadata()?; + } + self.created.insert(kind); Ok(()) } -} -/// In addition to checking the checksum, we also keep track of which files are open, and we delete -/// the open files if the program is terminated halfway. -pub struct ChecksumWriter { - writer: T, - path: PathBuf, - adler: adler::Adler32, -} + /// Lazily create `qi/` and `qi/n{n}_s{s}/` groups under this group prefix. + fn ensure_qi_bidegree(&self, b: Bidegree) -> anyhow::Result<()> { + let qi_root = self.group_fs_path().join("qi"); + if !qi_root.join("zarr.json").exists() { + GroupBuilder::new() + .build(self.store.clone(), &format!("{}/qi", self.group))? + .store_metadata()?; + } + let bidegree_meta = qi_root + .join(format!("n{}_s{}", b.n(), b.s())) + .join("zarr.json"); + if !bidegree_meta.exists() { + GroupBuilder::new() + .build( + self.store.clone(), + &format!("{}/qi/n{}_s{}", self.group, b.n(), b.s()), + )? + .store_metadata()?; + } + Ok(()) + } -impl ChecksumWriter { - pub fn new(path: PathBuf, writer: T) -> Self { - Self { - path, - writer, - adler: adler::Adler32::new(), + /// Write a sharded byte payload. + /// + /// Streamed kinds ([`SaveKind::ResQi`], [`SaveKind::NassauQi`]) are not supported here — use + /// [`Self::nassau_qi_writer`] / [`Self::write_res_qi`] instead, since they may be multi-GB + /// and would OOM the in-memory `Vec`. + /// + /// `location` is anything that implements [`SaveCoords`] — [`Bidegree`] for 2D kinds and + /// [`BidegreeGenerator`] for 3D kinds. Negative `n` is fine; the offset is handled + /// internally. + pub fn write( + &self, + kind: SaveKind, + location: impl SaveCoords, + data: &[u8], + ) -> anyhow::Result<()> { + debug_assert!( + !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), + "write() is only for sharded kinds, got {:?}", + kind + ); + self.ensure_shard_array(kind)?; + let lock = self.write_lock(kind); + let _guard = lock.lock().unwrap(); + let arr = zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind))?; + let zarr_coords = Self::shard_zarr_coords(location.save_coords()); + let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; + // Force sequential codec execution. Holding our std::sync::Mutex across + // `store_array_subset` is unsafe with rayon, because zarrs's sharding codec uses rayon + // internally — the worker that holds the mutex would join on inner tasks and could be + // assigned a new task that also needs the mutex, deadlocking. Sequential execution + // avoids the join entirely. + arr.store_array_subset_opt( + &subset, + vec![data.to_vec()], + &CodecOptions::default().with_concurrent_target(1), + ) + .map_err(zarr_err)?; + Ok(()) + } + + /// Read a sharded byte payload. + /// + /// Returns `None` if no data has been written. [`SaveKind::ResQi`] / [`SaveKind::NassauQi`] + /// are not supported here; use the dedicated per-kind APIs. + pub fn read( + &self, + kind: SaveKind, + location: impl SaveCoords, + ) -> anyhow::Result>> { + debug_assert!( + !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), + "read() is only for sharded kinds, got {:?}", + kind + ); + let arr = match zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind)) + { + Ok(arr) => arr, + Err(_) => return Ok(None), + }; + let zarr_coords = Self::shard_zarr_coords(location.save_coords()); + let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; + let data: Vec> = arr.retrieve_array_subset(&subset).map_err(zarr_err)?; + match data.into_iter().next() { + Some(element) if !element.is_empty() => Ok(Some(element)), + _ => Ok(None), } } -} -/// We only implement the functions required and the ones we actually use. -impl io::Write for ChecksumWriter { - fn write(&mut self, buf: &[u8]) -> io::Result { - let bytes_written = self.writer.write(buf)?; - self.adler.write_slice(&buf[0..bytes_written]); - Ok(bytes_written) + /// Check if a sharded payload exists. + pub fn exists(&self, kind: SaveKind, location: impl SaveCoords) -> bool { + debug_assert!( + !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), + "exists() is only for sharded kinds, got {:?}", + kind + ); + matches!(self.read(kind, location), Ok(Some(_))) + } + + /// Delete a sharded payload (overwrites with the empty fill value). + pub fn delete( + &self, + kind: SaveKind, + location: impl SaveCoords, + ) -> anyhow::Result<()> { + debug_assert!( + !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), + "delete() is only for sharded kinds, got {:?}", + kind + ); + // Overwrite with fill value (empty vec). The array must already exist for delete to be + // meaningful. Same locking + sequential codec story as `write`. + let lock = self.write_lock(kind); + let _guard = lock.lock().unwrap(); + let arr = zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind))?; + let zarr_coords = Self::shard_zarr_coords(location.save_coords()); + let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; + arr.store_array_subset_opt( + &subset, + vec![Vec::::new()], + &CodecOptions::default().with_concurrent_target(1), + ) + .map_err(zarr_err)?; + Ok(()) } - fn flush(&mut self) -> io::Result<()> { - self.writer.flush() + // --- ResQi structured I/O --- + + /// Filesystem path of the ResQi group for bidegree `b`. + fn res_qi_group_path(&self, b: Bidegree) -> String { + format!("{}/qi/n{}_s{}/res_qi", self.group, b.n(), b.s()) } - fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { - self.writer.write_all(buf)?; - self.adler.write_slice(buf); + /// Write a [`QuasiInverse`] as a structured zarr group at `qi/n{n}_s{s}/res_qi`. + /// + /// Layout: + /// + /// - group attributes: `source_dim`, `target_dim`, `image_dim`, `finished` + /// - `pivots`: 1D `i64`, shape `[target_dim]` + /// - `rows`: 2D `u8`, shape `[image_dim, num_limbs * 8]`, chunked over rows + /// + /// `finished` is set to `true` only on success, so a writer that crashes mid-write leaves + /// the group in a state the matching reader treats as missing. + pub fn write_res_qi(&self, b: Bidegree, qi: &QuasiInverse) -> anyhow::Result<()> { + self.ensure_qi_bidegree(b)?; + let group_path = self.res_qi_group_path(b); + + let source_dim = qi.source_dimension(); + let target_dim = qi.target_dimension(); + let image_dim = qi.image_dimension(); + let preimage = qi.preimage(); + let p = preimage.prime(); + let num_limbs = FpVector::num_limbs(p, source_dim); + let row_bytes = num_limbs * 8; + + // Group with attributes (finished = false until we're done) + let mut group_attrs = serde_json::Map::new(); + group_attrs.insert("source_dim".into(), (source_dim as u64).into()); + group_attrs.insert("target_dim".into(), (target_dim as u64).into()); + group_attrs.insert("image_dim".into(), (image_dim as u64).into()); + group_attrs.insert("finished".into(), false.into()); + let group = GroupBuilder::new() + .attributes(group_attrs.clone()) + .build(self.store.clone(), &group_path)?; + group.store_metadata()?; + + // pivots array: 1D i64, single chunk. + let pivots_shape = std::cmp::max(target_dim as u64, 1); + let pivots_array = ArrayBuilder::new( + vec![pivots_shape], + vec![pivots_shape], + data_type::int64(), + zarrs::array::FillValue::from(0i64), + ) + .bytes_to_bytes_codecs(stream_tier_codecs()) + .build(self.store.clone(), &format!("{}/pivots", group_path))?; + pivots_array.store_metadata()?; + let mut pivots_data: Vec = match qi.pivots() { + Some(p) => p.iter().map(|&x| x as i64).collect(), + None => (0..target_dim as i64).collect(), + }; + if pivots_data.is_empty() { + pivots_data.push(0); + } + pivots_array + .store_chunk(&[0], pivots_data) + .map_err(zarr_err)?; + + // rows array: 2D u8 [image_dim, row_bytes], chunked over rows. + let rows_shape_0 = std::cmp::max(image_dim as u64, 1); + let rows_shape_1 = std::cmp::max(row_bytes as u64, 1); + let chunk_rows = std::cmp::min(CHUNK_RES_QI_ROWS, rows_shape_0); + let rows_array = ArrayBuilder::new( + vec![rows_shape_0, rows_shape_1], + vec![chunk_rows, rows_shape_1], + data_type::uint8(), + zarrs::array::FillValue::from(0u8), + ) + .bytes_to_bytes_codecs(stream_tier_codecs()) + .build(self.store.clone(), &format!("{}/rows", group_path))?; + rows_array.store_metadata()?; + + if image_dim > 0 && row_bytes > 0 { + // Write rows in chunks of `chunk_rows` rows. Pad the last chunk with zeros so the + // chunk-shape constraint is satisfied; the reader knows `image_dim` and ignores the + // padded rows. + let chunk_byte_len = (chunk_rows as usize) * row_bytes; + let mut chunk_buf: Vec = Vec::with_capacity(chunk_byte_len); + let mut chunk_idx: u64 = 0; + for row_idx in 0..image_dim { + let row_vec: FpVector = preimage.row(row_idx).to_owned(); + let buf_before = chunk_buf.len(); + row_vec.to_bytes(&mut chunk_buf)?; + debug_assert_eq!(chunk_buf.len() - buf_before, row_bytes); + if chunk_buf.len() == chunk_byte_len { + rows_array + .store_chunk(&[chunk_idx, 0], std::mem::take(&mut chunk_buf)) + .map_err(zarr_err)?; + chunk_idx += 1; + chunk_buf.reserve(chunk_byte_len); + } + } + if !chunk_buf.is_empty() { + chunk_buf.resize(chunk_byte_len, 0); + rows_array + .store_chunk(&[chunk_idx, 0], chunk_buf) + .map_err(zarr_err)?; + } + } + + // Mark the group finished. + group_attrs.insert("finished".into(), true.into()); + let finished_group = GroupBuilder::new() + .attributes(group_attrs) + .build(self.store.clone(), &group_path)?; + finished_group.store_metadata()?; Ok(()) } -} -impl std::ops::Drop for ChecksumWriter { - fn drop(&mut self) { - if !std::thread::panicking() { - // We may not have finished writing, so the data is wrong. It should not be given a - // valid checksum - self.writer - .write_u32::(self.adler.checksum()) - .unwrap(); - self.writer.flush().unwrap(); - assert!( - open_files().lock().unwrap().remove(&self.path), - "File {:?} already dropped", - self.path - ); + /// Open a streaming reader for the ResQi at bidegree `b`. + /// + /// Returns `None` if no finished group exists. The reader fetches one chunk of rows at a + /// time so peak memory is bounded by `CHUNK_RES_QI_ROWS * row_bytes`. + pub fn stream_res_qi(&self, b: Bidegree, p: ValidPrime) -> anyhow::Result> { + let group_path = self.res_qi_group_path(b); + let group = match zarrs::group::Group::open(self.store.clone(), &group_path) { + Ok(g) => g, + Err(_) => return Ok(None), + }; + let attrs = group.attributes(); + if !attrs + .get("finished") + .and_then(|v| v.as_bool()) + .unwrap_or(false) + { + return Ok(None); } - tracing::info!(file = ?self.path, "closing"); + let source_dim = attrs + .get("source_dim") + .and_then(|v| v.as_u64()) + .unwrap_or(0) as usize; + let target_dim = attrs + .get("target_dim") + .and_then(|v| v.as_u64()) + .unwrap_or(0) as usize; + let image_dim = attrs.get("image_dim").and_then(|v| v.as_u64()).unwrap_or(0) as usize; + + let pivots_array = + zarrs::array::Array::open(self.store.clone(), &format!("{}/pivots", group_path))?; + let pivots_chunk: Vec = pivots_array.retrieve_chunk(&[0]).map_err(zarr_err)?; + let pivots: Vec = pivots_chunk + .into_iter() + .take(target_dim) + .map(|x| x as isize) + .collect(); + + let rows_array = + zarrs::array::Array::open(self.store.clone(), &format!("{}/rows", group_path))?; + + Ok(Some(ResQiReader { + p, + source_dim, + target_dim, + image_dim, + pivots, + rows_array, + chunk_buf: Vec::new(), + chunk_idx: 0, + pos_in_chunk: 0, + row_bytes: FpVector::num_limbs(p, source_dim) * 8, + chunk_rows: 0, + })) } -} -pub struct ChecksumReader { - reader: T, - adler: adler::Adler32, -} + // --- NassauQi structured I/O --- + + /// Filesystem path of the NassauQi group for bidegree `b`. + fn nassau_qi_group_path(&self, b: Bidegree) -> String { + format!("{}/qi/n{}_s{}/nassau_qi", self.group, b.n(), b.s()) + } + + /// Open a writer for a NassauQi at bidegree `b`. + /// + /// The header (target/zero mask dimensions and the subalgebra profile bytes) goes into group + /// attributes; the body of the bytecode becomes a 1D vlen-bytes `commands` array, with each + /// element holding one command. + pub fn nassau_qi_writer( + &self, + b: Bidegree, + target_dim: u64, + zero_mask_dim: u64, + subalgebra_profile: &[u8], + ) -> anyhow::Result { + self.ensure_qi_bidegree(b)?; + let group_path = self.nassau_qi_group_path(b); + + let mut group_attrs = serde_json::Map::new(); + group_attrs.insert("target_dim".into(), target_dim.into()); + group_attrs.insert("zero_mask_dim".into(), zero_mask_dim.into()); + group_attrs.insert( + "subalgebra_profile".into(), + serde_json::Value::Array( + subalgebra_profile + .iter() + .map(|&b| serde_json::Value::from(b as u64)) + .collect(), + ), + ); + group_attrs.insert("finished".into(), false.into()); + let group = GroupBuilder::new() + .attributes(group_attrs.clone()) + .build(self.store.clone(), &group_path)?; + group.store_metadata()?; + + let commands_array = ArrayBuilder::new( + vec![NASSAU_QI_MAX_COMMANDS], + vec![CHUNK_NASSAU_COMMANDS], + data_type::bytes(), + zarrs::array::FillValue::from(Vec::::new()), + ) + .bytes_to_bytes_codecs(stream_tier_codecs()) + .build(self.store.clone(), &format!("{}/commands", group_path))?; + commands_array.store_metadata()?; + + Ok(NassauQiWriter { + store: self.store.clone(), + group_path, + group_attrs, + commands_array, + command_buf: Vec::with_capacity(CHUNK_NASSAU_COMMANDS as usize), + chunk_idx: 0, + commands_written: 0, + }) + } -impl ChecksumReader { - pub fn new(reader: T) -> Self { - Self { - reader, - adler: adler::Adler32::new(), + /// Open a streaming reader for the NassauQi at bidegree `b`. + /// + /// Returns `None` if no finished group exists. The reader yields one [`NassauCommand`] at a + /// time and fetches one chunk of commands at a time, so peak memory is bounded by + /// `CHUNK_NASSAU_COMMANDS * avg_command_bytes`. + pub fn nassau_qi_reader(&self, b: Bidegree) -> anyhow::Result> { + let group_path = self.nassau_qi_group_path(b); + let group = match zarrs::group::Group::open(self.store.clone(), &group_path) { + Ok(g) => g, + Err(_) => return Ok(None), + }; + let attrs = group.attributes(); + if !attrs + .get("finished") + .and_then(|v| v.as_bool()) + .unwrap_or(false) + { + return Ok(None); } + let target_dim = attrs + .get("target_dim") + .and_then(|v| v.as_u64()) + .unwrap_or(0); + let zero_mask_dim = attrs + .get("zero_mask_dim") + .and_then(|v| v.as_u64()) + .unwrap_or(0); + let num_commands = attrs + .get("num_commands") + .and_then(|v| v.as_u64()) + .unwrap_or(0); + let subalgebra_profile: Vec = attrs + .get("subalgebra_profile") + .and_then(|v| v.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|x| x.as_u64().map(|n| n as u8)) + .collect() + }) + .unwrap_or_default(); + + let commands_array = + zarrs::array::Array::open(self.store.clone(), &format!("{}/commands", group_path))?; + + Ok(Some(NassauQiReader { + target_dim, + zero_mask_dim, + subalgebra_profile, + commands_array, + num_commands, + consumed: 0, + chunk_buf: Vec::new(), + pos_in_chunk: 0, + chunk_idx: 0, + })) } } -/// We only implement the functions required and the ones we actually use. -impl io::Read for ChecksumReader { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - let bytes_read = self.reader.read(buf)?; - self.adler.write_slice(&buf[0..bytes_read]); - Ok(bytes_read) +// --- ResQi reader --- + +/// Streaming reader for a structured ResQi. +/// +/// Reads matrix rows from the underlying chunked 2D `rows` array on demand. +pub struct ResQiReader { + p: ValidPrime, + source_dim: usize, + target_dim: usize, + image_dim: usize, + pivots: Vec, + rows_array: zarrs::array::Array, + /// Current chunk's flat byte buffer. + chunk_buf: Vec, + /// Index of the next chunk to fetch. + chunk_idx: u64, + /// Position within `chunk_buf`, in rows (not bytes). + pos_in_chunk: usize, + /// Bytes per row, computed at construction. + row_bytes: usize, + /// Number of rows per chunk; cached on first chunk fetch. + chunk_rows: usize, +} + +impl ResQiReader { + pub fn source_dimension(&self) -> usize { + self.source_dim } - fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> { - self.reader.read_exact(buf)?; - self.adler.write_slice(buf); - Ok(()) + pub fn target_dimension(&self) -> usize { + self.target_dim } -} -impl std::ops::Drop for ChecksumReader { - fn drop(&mut self) { - if !std::thread::panicking() { - // If we are panicking, we may not have read everything, and panic in panic - // is bad. - assert_eq!( - self.adler.checksum(), - self.reader.read_u32::().unwrap(), - "Invalid file checksum" - ); - let mut buf = [0]; - // Check EOF - assert_eq!(self.reader.read(&mut buf).unwrap(), 0, "EOF not reached"); - } + pub fn image_dimension(&self) -> usize { + self.image_dim } -} -/// Open the file pointed to by `path` as a `Box`. If the file does not exist, look for -/// compressed versions. -fn open_file(path: PathBuf) -> Option> { - use io::BufRead; - - // We should try in decreasing order of access speed. - match File::open(&path) { - Ok(f) => { - let mut reader = io::BufReader::new(f); - if reader - .fill_buf() - .unwrap_or_else(|e| panic!("Error when reading from {path:?}: {e}")) - .is_empty() - { - // The file is empty. Delete the file and proceed as if it didn't exist - std::fs::remove_file(&path) - .unwrap_or_else(|e| panic!("Error when deleting empty file {path:?}: {e}")); - return None; - } - return Some(Box::new(ChecksumReader::new(reader))); + pub fn pivots(&self) -> &[isize] { + &self.pivots + } + + /// Read the next matrix row into `dest`. + /// + /// Returns whether a row was read. Rows are returned in the order they were written (one per + /// non-trivial pivot column). + pub fn next_row(&mut self, dest: &mut FpVector) -> anyhow::Result { + if self.image_dim == 0 || self.row_bytes == 0 { + return Ok(false); + } + let total_rows_consumed = + (self.chunk_idx.saturating_sub(1) as usize) * self.chunk_rows + self.pos_in_chunk; + if total_rows_consumed >= self.image_dim { + return Ok(false); } - Err(e) => { - if e.kind() != io::ErrorKind::NotFound { - panic!("Error when opening {path:?}: {e}"); + if self.chunk_buf.is_empty() || self.pos_in_chunk * self.row_bytes >= self.chunk_buf.len() { + // Refill from the next chunk. + let chunk: Vec = self + .rows_array + .retrieve_chunk(&[self.chunk_idx, 0]) + .map_err(zarr_err)?; + // Cache rows-per-chunk on first fetch. + if self.chunk_rows == 0 && self.row_bytes > 0 { + self.chunk_rows = chunk.len() / self.row_bytes; } + self.chunk_buf = chunk; + self.pos_in_chunk = 0; + self.chunk_idx += 1; } + let start = self.pos_in_chunk * self.row_bytes; + let end = start + self.row_bytes; + dest.update_from_bytes(&mut &self.chunk_buf[start..end])?; + self.pos_in_chunk += 1; + Ok(true) } - #[cfg(feature = "zstd")] + /// Apply this quasi-inverse to all the vectors in `inputs`, accumulating the results into + /// `results`. + /// + /// Mirrors the semantics of the legacy `QuasiInverse::stream_quasi_inverse` but reads from + /// the structured zarr layout. + pub fn apply(mut self, results: &mut [T], inputs: &[S]) -> anyhow::Result<()> + where + for<'a> &'a mut T: Into>, + for<'a> &'a S: Into>, { - let mut path = path; - path.set_extension("zst"); - match File::open(&path) { - Ok(f) => { - return Some(Box::new(ChecksumReader::new( - zstd::stream::Decoder::new(f).unwrap(), - ))); + use itertools::Itertools; + assert_eq!(results.len(), inputs.len()); + let mut row = FpVector::new(self.p, self.source_dim); + let pivots = self.pivots.clone(); + for (i, &r) in pivots.iter().enumerate() { + if r < 0 { + continue; } - Err(e) => { - if e.kind() != io::ErrorKind::NotFound { - panic!("Error when opening {path:?}"); - } + let got = self.next_row(&mut row)?; + assert!(got, "ResQi truncated: expected row for pivot {i}"); + for (input, result) in inputs.iter().zip_eq(results.iter_mut()) { + result.into().add(row.as_slice(), input.into().entry(i)); } } + Ok(()) + } + + /// Materialize the full [`QuasiInverse`] in memory. + /// + /// Used by the load-on-resume path; for streaming application, prefer [`Self::apply`]. + pub fn into_quasi_inverse(mut self) -> anyhow::Result { + let mut rows: Vec = Vec::with_capacity(self.image_dim); + for _ in 0..self.image_dim { + let mut row = FpVector::new(self.p, self.source_dim); + let got = self.next_row(&mut row)?; + assert!(got, "ResQi truncated while materializing"); + rows.push(row); + } + let preimage = Matrix::from_rows(self.p, rows, self.source_dim); + Ok(QuasiInverse::new(Some(self.pivots), preimage)) } +} + +// --- NassauQi writer/reader --- - None +/// One command in a NassauQi command stream. +/// +/// Mirrors the original bytecode but as discrete typed values instead of an inline `i64` magic +/// number stream. +#[derive(Debug, Clone)] +pub enum NassauCommand { + /// Switch to a new subalgebra signature. + /// + /// Subsequent pivot lifts are expressed in the masked basis under this signature. + Signature(Vec), + /// "Differential fix" — emitted (at most once) at the end of the zero-signature section + /// when the bidegree was resolved through stem rather than through `t`. + /// + /// Carries no payload. + Fix, + /// A pivot column with its lift and image. + /// + /// `lift_bytes` and `image_bytes` are raw `FpVector` limb serialisations; the caller knows + /// the dimensions from the current signature state and `target_dim`. + Pivot { + col: u64, + lift_bytes: Vec, + image_bytes: Vec, + }, } -pub struct SaveFile { - pub kind: SaveKind, - pub algebra: Arc, - pub b: Bidegree, - pub idx: Option, +const NASSAU_CODE_SIGNATURE: i64 = -2; +const NASSAU_CODE_FIX: i64 = -3; + +/// Writer for a structured NassauQi. +/// +/// Each call to a `write_*` method appends one command to the in-memory buffer; the buffer is +/// flushed to the underlying zarr `commands` array when `CHUNK_NASSAU_COMMANDS` commands have +/// accumulated. `finish()` flushes any remaining commands and commits the `num_commands` and +/// `finished` group attributes. +pub struct NassauQiWriter { + store: ReadableWritableListableStorage, + group_path: String, + group_attrs: serde_json::Map, + commands_array: zarrs::array::Array, + command_buf: Vec>, + chunk_idx: u64, + commands_written: u64, } -impl SaveFile { - fn write_header(&self, buffer: &mut impl io::Write) -> io::Result<()> { - buffer.write_u32::(self.kind.magic())?; - buffer.write_u32::(self.algebra.magic())?; - buffer.write_i32::(self.b.s())?; - buffer.write_i32::(if let Some(i) = self.idx { - self.b.t() + ((i as i32) << 16) - } else { - self.b.t() - }) +impl NassauQiWriter { + fn add_command(&mut self, bytes: Vec) -> anyhow::Result<()> { + self.command_buf.push(bytes); + self.commands_written += 1; + if self.command_buf.len() == CHUNK_NASSAU_COMMANDS as usize { + self.flush_chunk(false)?; + } + Ok(()) } - fn validate_header(&self, buffer: &mut impl io::Read) -> io::Result<()> { - macro_rules! check_header { - ($name:literal, $value:expr, $format:literal) => { - let data = buffer.read_u32::()?; - if data != $value { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Invalid header: {} was {} but expected {}", - $name, - format_args!($format, data), - format_args!($format, $value) - ), - )); - } - }; + fn flush_chunk(&mut self, pad: bool) -> anyhow::Result<()> { + if self.command_buf.is_empty() { + return Ok(()); + } + let mut chunk = std::mem::take(&mut self.command_buf); + if pad { + // Pad to chunk shape with empty elements; the reader knows num_commands and ignores + // them. + chunk.resize_with(CHUNK_NASSAU_COMMANDS as usize, Vec::new); } + self.commands_array + .store_chunk(&[self.chunk_idx], chunk) + .map_err(zarr_err)?; + self.chunk_idx += 1; + self.command_buf.reserve(CHUNK_NASSAU_COMMANDS as usize); + Ok(()) + } - check_header!("magic", self.kind.magic(), "{:#010x}"); - check_header!("algebra", self.algebra.magic(), "{:#06x}"); - check_header!("s", self.b.s() as u32, "{}"); - check_header!( - "t", - if let Some(i) = self.idx { - self.b.t() as u32 + ((i as u32) << 16) - } else { - self.b.t() as u32 - }, - "{}" - ); + pub fn write_signature(&mut self, signature: &[u16]) -> anyhow::Result<()> { + let mut bytes = Vec::with_capacity(8 + signature.len() * 2); + bytes.extend_from_slice(&NASSAU_CODE_SIGNATURE.to_le_bytes()); + for &x in signature { + bytes.extend_from_slice(&x.to_le_bytes()); + } + self.add_command(bytes) + } + pub fn write_fix(&mut self) -> anyhow::Result<()> { + let bytes = NASSAU_CODE_FIX.to_le_bytes().to_vec(); + self.add_command(bytes) + } + + pub fn write_pivot(&mut self, col: u64, lift: FpSlice, image: FpSlice) -> anyhow::Result<()> { + let lift_vec: FpVector = lift.to_owned(); + let mut lift_bytes = Vec::new(); + lift_vec.to_bytes(&mut lift_bytes)?; + let image_vec: FpVector = image.to_owned(); + let mut image_bytes = Vec::new(); + image_vec.to_bytes(&mut image_bytes)?; + + let mut bytes = Vec::with_capacity(8 + 4 + lift_bytes.len() + image_bytes.len()); + bytes.extend_from_slice(&(col as i64).to_le_bytes()); + bytes.extend_from_slice(&(lift_bytes.len() as u32).to_le_bytes()); + bytes.extend_from_slice(&lift_bytes); + bytes.extend_from_slice(&image_bytes); + self.add_command(bytes) + } + + /// Finalize: flush the partial last chunk and commit the `num_commands` and + /// `finished = true` attributes. + pub fn finish(mut self) -> anyhow::Result<()> { + let total = self.commands_written; + self.flush_chunk(true)?; + self.group_attrs.insert("num_commands".into(), total.into()); + self.group_attrs.insert("finished".into(), true.into()); + let group = GroupBuilder::new() + .attributes(self.group_attrs) + .build(self.store.clone(), &self.group_path)?; + group.store_metadata()?; Ok(()) } +} - /// This panics if there is no save dir - fn get_save_path(&self, mut dir: PathBuf) -> PathBuf { - if let Some(idx) = self.idx { - dir.push(format!( - "{name}s/{s}_{t}_{idx}_{name}", - name = self.kind.name(), - s = self.b.s(), - t = self.b.t() - )); - } else { - dir.push(format!( - "{name}s/{s}_{t}_{name}", - name = self.kind.name(), - s = self.b.s(), - t = self.b.t() - )); - } - dir +/// Streaming reader for a structured NassauQi. +/// +/// Yields one [`NassauCommand`] at a time, fetching chunks from the underlying `commands` array +/// on demand. +pub struct NassauQiReader { + target_dim: u64, + zero_mask_dim: u64, + subalgebra_profile: Vec, + commands_array: zarrs::array::Array, + num_commands: u64, + consumed: u64, + chunk_buf: Vec>, + pos_in_chunk: usize, + chunk_idx: u64, +} + +impl NassauQiReader { + pub fn target_dim(&self) -> u64 { + self.target_dim } - pub fn open_file(&self, dir: PathBuf) -> Option> { - let file_path = self.get_save_path(dir); - let path_string = file_path.to_string_lossy().into_owned(); - if let Some(mut f) = open_file(file_path) { - self.validate_header(&mut f).unwrap(); - tracing::info!(file = path_string, "success open for reading"); - Some(f) - } else { - tracing::info!(file = path_string, "failed open for reading"); - None + pub fn zero_mask_dim(&self) -> u64 { + self.zero_mask_dim + } + + pub fn subalgebra_profile(&self) -> &[u8] { + &self.subalgebra_profile + } + + fn parse(bytes: Vec) -> anyhow::Result { + if bytes.len() < 8 { + anyhow::bail!("NassauQi command too short: {} bytes", bytes.len()); + } + let code = i64::from_le_bytes(bytes[..8].try_into().unwrap()); + match code { + NASSAU_CODE_SIGNATURE => { + let payload = &bytes[8..]; + let sig: Vec = payload + .chunks_exact(2) + .map(|c| u16::from_le_bytes([c[0], c[1]])) + .collect(); + Ok(NassauCommand::Signature(sig)) + } + NASSAU_CODE_FIX => Ok(NassauCommand::Fix), + col if col >= 0 => { + if bytes.len() < 12 { + anyhow::bail!("NassauQi pivot command too short: {} bytes", bytes.len()); + } + let lift_byte_len = u32::from_le_bytes(bytes[8..12].try_into().unwrap()) as usize; + if bytes.len() < 12 + lift_byte_len { + anyhow::bail!( + "NassauQi pivot command lift truncated: have {}, need {}", + bytes.len(), + 12 + lift_byte_len + ); + } + let lift_bytes = bytes[12..12 + lift_byte_len].to_vec(); + let image_bytes = bytes[12 + lift_byte_len..].to_vec(); + Ok(NassauCommand::Pivot { + col: col as u64, + lift_bytes, + image_bytes, + }) + } + _ => anyhow::bail!("Unknown NassauQi command code: {code}"), } } +} + +impl Iterator for NassauQiReader { + type Item = anyhow::Result; - pub fn exists(&self, dir: PathBuf) -> bool { - let path = self.get_save_path(dir); - if path.exists() { - return true; + fn next(&mut self) -> Option { + if self.consumed >= self.num_commands { + return None; } - #[cfg(not(target_arch = "wasm32"))] - { - let mut path = path; - path.set_extension("zst"); - if path.exists() { - return true; + if self.pos_in_chunk >= self.chunk_buf.len() { + match self.commands_array.retrieve_chunk(&[self.chunk_idx]) { + Ok(buf) => self.chunk_buf = buf, + Err(e) => return Some(Err(zarr_err(e))), } + self.chunk_idx += 1; + self.pos_in_chunk = 0; } - false + let bytes = std::mem::take(&mut self.chunk_buf[self.pos_in_chunk]); + self.pos_in_chunk += 1; + self.consumed += 1; + Some(Self::parse(bytes)) } - pub fn delete_file(&self, dir: PathBuf) -> io::Result<()> { - let p = self.get_save_path(dir); - match std::fs::remove_file(p) { - Ok(()) => Ok(()), - Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), - Err(e) => Err(e), + fn size_hint(&self) -> (usize, Option) { + let remaining = (self.num_commands - self.consumed) as usize; + (remaining, Some(remaining)) + } +} + +// --- SaveCoords, SaveKind, SaveDirectory --- + +/// Anything that names a save location, parameterised by its dimension `N`. +/// +/// [`Bidegree`] is `SaveCoords<2>` and [`BidegreeGenerator`] is `SaveCoords<3>`, so methods that +/// only make sense in one of those dimensions (e.g. the stream-tier writer / reader, which are +/// per-bidegree) can take `impl SaveCoords<2>` and refuse the other type at compile time. The +/// store reads `(n, s, [idx])` from this, shifts `n` by the internal `N_MIN` offset, and uses +/// the result as a zarr index. Implementing this trait for `MultiDegree` / +/// `MultiDegreeGenerator` would extend the same API to higher-`N` gradings without further +/// changes here. +pub trait SaveCoords { + fn save_coords(&self) -> [i32; N]; +} + +impl SaveCoords<2> for Bidegree { + fn save_coords(&self) -> [i32; 2] { + [self.n(), self.s()] + } +} + +impl SaveCoords<3> for BidegreeGenerator { + fn save_coords(&self) -> [i32; 3] { + [self.n(), self.s(), self.idx() as i32] + } +} + +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +#[non_exhaustive] +pub enum SaveKind { + Kernel, + Differential, + ResQi, + AugmentationQi, + SecondaryComposite, + SecondaryIntermediate, + SecondaryHomotopy, + ChainMap, + ChainHomotopy, + NassauDifferential, + NassauQi, +} + +impl SaveKind { + pub fn name(self) -> &'static str { + match self { + Self::Kernel => "kernel", + Self::Differential => "differential", + Self::ResQi => "res_qi", + Self::AugmentationQi => "augmentation_qi", + Self::SecondaryComposite => "secondary_composite", + Self::SecondaryIntermediate => "secondary_intermediate", + Self::SecondaryHomotopy => "secondary_homotopy", + Self::ChainMap => "chain_map", + Self::ChainHomotopy => "chain_homotopy", + Self::NassauDifferential => "nassau_differential", + Self::NassauQi => "nassau_qi", } } - /// # Arguments - /// - `overwrite`: Whether to overwrite a file if it already exists. - pub fn create_file(&self, dir: PathBuf, overwrite: bool) -> impl io::Write + use { - let p = self.get_save_path(dir); - tracing::info!(file = ?p, "open for writing"); + /// Whether this kind uses 3D indexing `(n, s, idx)`. + /// + /// The third coordinate is an intra-lift enumeration over basis elements; multi-hom + /// disambiguation is handled by the store's group prefix, not by an extra coordinate. + /// `SecondaryHomotopy` is per-bidegree (data for all generators is concatenated), so it + /// stays 2D. + fn is_indexed(self) -> bool { + matches!(self, Self::SecondaryComposite | Self::SecondaryIntermediate) + } - // We need to do this before creating any file. The ctrlc handler does not block other threads - // from running, but it does lock [`open_files()`]. So this ensures we do not open new files - // while handling ctrlc. - assert!( - open_files().lock().unwrap().insert(p.clone()), - "File {p:?} is already opened" - ); + pub fn resolution_data() -> impl Iterator { + use SaveKind::*; + static KINDS: [SaveKind; 4] = [Kernel, Differential, ResQi, AugmentationQi]; + KINDS.iter().copied() + } + + pub fn nassau_data() -> impl Iterator { + use SaveKind::*; + static KINDS: [SaveKind; 2] = [NassauDifferential, NassauQi]; + KINDS.iter().copied() + } - let f = std::fs::OpenOptions::new() - .write(true) - .create_new(!overwrite) - .create(true) - .truncate(true) - .open(&p) - .with_context(|| format!("Failed to create save file {p:?}")) - .unwrap(); - let mut f = ChecksumWriter::new(p, io::BufWriter::new(f)); - self.write_header(&mut f).unwrap(); - f + pub fn secondary_data() -> impl Iterator { + use SaveKind::*; + static KINDS: [SaveKind; 3] = + [SecondaryComposite, SecondaryIntermediate, SecondaryHomotopy]; + KINDS.iter().copied() } } -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct AbsolutePath(PathBuf); +#[derive(Debug)] +pub enum SaveDirectory { + None, + Store(ZarrSaveStore), +} -impl AbsolutePath { - pub fn new>(path: P) -> Self { - let p = std::path::absolute(path) - .unwrap_or_else(|e| panic!("Error when getting absolute path: {e}")); - Self(p) +impl SaveDirectory { + pub fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + pub fn is_some(&self) -> bool { + !self.is_none() } - pub fn push>(&mut self, p: P) { - self.0.push(p); + pub fn store(&self) -> Option<&ZarrSaveStore> { + match self { + Self::None => None, + Self::Store(s) => Some(s), + } } } -impl> From

for AbsolutePath { - fn from(p: P) -> Self { - Self::new(p) +impl From> for SaveDirectory { + fn from(x: Option) -> Self { + match x { + None => Self::None, + Some(p) => Self::Store(ZarrSaveStore::create(p).expect("Failed to create zarr store")), + } } } diff --git a/ext/src/secondary.rs b/ext/src/secondary.rs index e04b852a36..9506f49c61 100644 --- a/ext/src/secondary.rs +++ b/ext/src/secondary.rs @@ -34,7 +34,7 @@ pub use crate::{ }; use crate::{ chain_complex::{ChainComplex, FreeChainComplex}, - save::{SaveDirectory, SaveFile, SaveKind}, + save::{SaveDirectory, SaveKind}, }; pub static LAMBDA_BIDEGREE: Bidegree = Bidegree::n_s(0, 1); @@ -253,20 +253,14 @@ impl SecondaryHomotopy { let f = |t, idx| { let _tracing_guard = tracing_span.enter(); let g = BidegreeGenerator::s_t(s, t, idx); - let save_file = SaveFile { - algebra: self.target.algebra(), - kind: SaveKind::SecondaryComposite, - b: g.degree(), - idx: Some(g.idx()), - }; - if let Some(dir) = dir.read() - && let Some(mut f) = save_file.open_file(dir.to_owned()) + if let Some(store) = dir.store() + && let Some(data) = store.read(SaveKind::SecondaryComposite, g).unwrap() { return SecondaryComposite::from_bytes( Arc::clone(&self.target), g.t() - self.shift_t, self.hit_generator, - &mut f, + &mut &data[..], ) .unwrap(); } @@ -284,9 +278,10 @@ impl SecondaryHomotopy { composite.finalize(); }); - if let Some(dir) = dir.write() { - let mut f = save_file.create_file(dir.to_owned(), false); - composite.to_bytes(&mut f).unwrap(); + if let Some(store) = dir.store() { + let mut buf = Vec::new(); + composite.to_bytes(&mut buf).unwrap(); + store.write(SaveKind::SecondaryComposite, g, &buf).unwrap(); } composite @@ -427,27 +422,24 @@ pub trait SecondaryLift: Sync + Sized { return v; } - let save_file = SaveFile { - algebra: self.algebra(), - kind: SaveKind::SecondaryIntermediate, - b: g.degree(), - idx: Some(g.idx()), - }; - - if let Some(dir) = self.save_dir().read() - && let Some(mut f) = save_file.open_file(dir.to_owned()) + if let Some(store) = self.save_dir().store() + && let Some(data) = store.read(SaveKind::SecondaryIntermediate, g).unwrap() { // The target dimension can depend on whether we resolved to stem - let dim = f.read_u64::().unwrap() as usize; - return FpVector::from_bytes(self.prime(), dim, &mut f).unwrap(); + let mut cursor = &data[..]; + let dim = cursor.read_u64::().unwrap() as usize; + return FpVector::from_bytes(self.prime(), dim, &mut cursor).unwrap(); } let result = self.compute_intermediate(g); - if let Some(dir) = self.save_dir().write() { - let mut f = save_file.create_file(dir.to_owned(), false); - f.write_u64::(result.len() as u64).unwrap(); - result.to_bytes(&mut f).unwrap(); + if let Some(store) = self.save_dir().store() { + let mut buf = Vec::new(); + buf.write_u64::(result.len() as u64).unwrap(); + result.to_bytes(&mut buf).unwrap(); + store + .write(SaveKind::SecondaryIntermediate, g, &buf) + .unwrap(); } result @@ -494,17 +486,10 @@ pub trait SecondaryLift: Sync + Sized { return; } // Check if we have a saved homotopy - if let Some(dir) = self.save_dir().read() { - let save_file = SaveFile { - algebra: self.algebra(), - kind: SaveKind::SecondaryHomotopy, - b: g.degree(), - idx: None, - }; - - if save_file.exists(dir.to_owned()) { - return; - } + if let Some(store) = self.save_dir().store() + && store.exists(SaveKind::SecondaryHomotopy, g.degree()) + { + return; } self.intermediates().insert(g, self.get_intermediate(g)); }; @@ -550,23 +535,17 @@ pub trait SecondaryLift: Sync + Sized { let num_gens = source.number_of_gens_in_degree(b.t()); let target_dim = target.module(target_b.s()).dimension(target_b.t()); - if let Some(dir) = self.save_dir().read() { - let save_file = SaveFile { - algebra: self.algebra(), - kind: SaveKind::SecondaryHomotopy, - b, - idx: None, - }; - - if let Some(mut f) = save_file.open_file(dir.to_owned()) { - let mut results = Vec::with_capacity(num_gens); - for _ in 0..num_gens { - results.push(FpVector::from_bytes(p, target_dim, &mut f).unwrap()); - } - return Ok(self.homotopies()[b.s()] - .homotopies - .add_generators_from_rows_ooo(b.t(), results)); + if let Some(store) = self.save_dir().store() + && let Some(data) = store.read(SaveKind::SecondaryHomotopy, b).unwrap() + { + let mut cursor = &data[..]; + let mut results = Vec::with_capacity(num_gens); + for _ in 0..num_gens { + results.push(FpVector::from_bytes(p, target_dim, &mut cursor).unwrap()); } + return Ok(self.homotopies()[b.s()] + .homotopies + .add_generators_from_rows_ooo(b.t(), results)); } let tracing_span = tracing::Span::current(); @@ -610,30 +589,20 @@ pub trait SecondaryLift: Sync + Sized { } } - if let Some(dir) = self.save_dir().write() { - let save_file = SaveFile { - algebra: self.algebra(), - kind: SaveKind::SecondaryHomotopy, - b, - idx: None, - }; - - let mut f = save_file.create_file(dir.to_owned(), false); + if let Some(store) = self.save_dir().store() { + let mut buf = Vec::new(); for row in &results { - row.to_bytes(&mut f).unwrap(); + row.to_bytes(&mut buf).unwrap(); } - drop(f); - - let mut save_file = SaveFile { - algebra: self.algebra(), - kind: SaveKind::SecondaryIntermediate, - b, - idx: None, - }; + store.write(SaveKind::SecondaryHomotopy, b, &buf).unwrap(); for i in 0..num_gens { - save_file.idx = Some(i); - save_file.delete_file(dir.to_owned()).unwrap(); + store + .delete( + SaveKind::SecondaryIntermediate, + BidegreeGenerator::new(b, i), + ) + .unwrap(); } } diff --git a/ext/tests/save_load_resolution.rs b/ext/tests/save_load_resolution.rs index 556161514f..e32819f98c 100644 --- a/ext/tests/save_load_resolution.rs +++ b/ext/tests/save_load_resolution.rs @@ -1,69 +1,13 @@ -use std::{ - path::{Path, PathBuf}, - sync::Arc, -}; +use std::sync::Arc; use algebra::module::homomorphism::ModuleHomomorphism; use ext::{ chain_complex::{ChainComplex, FreeChainComplex}, - save::{SaveDirectory, SaveKind}, secondary::{SecondaryLift, SecondaryResolution}, utils::construct_standard, }; use sseq::coordinates::Bidegree; -fn set_readonly(p: &Path, readonly: bool) { - let mut perm = p.metadata().unwrap().permissions(); - perm.set_readonly(readonly); - std::fs::set_permissions(p, perm).unwrap(); -} - -fn lock_tempdir(dir: &Path) { - let mut dir: PathBuf = dir.into(); - for kind in SaveKind::resolution_data() { - dir.push(format!("{}s", kind.name())); - set_readonly(&dir, true); - dir.pop(); - } - set_readonly(&dir, true); -} - -/// Should unlock after the test so that cleanup can be performed -fn unlock_tempdir(dir: &Path) { - set_readonly(dir, false); - - let mut dir: PathBuf = dir.into(); - for kind in SaveKind::resolution_data() { - dir.push(format!("{}s", kind.name())); - set_readonly(&dir, false); - dir.pop(); - } -} - -#[test] -#[should_panic(expected = "Permission denied")] -fn test_tempdir_lock() { - let tempdir = tempfile::TempDir::new().unwrap(); - let resolution1 = - construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); - resolution1.compute_through_bidegree(Bidegree::s_t(5, 5)); - - lock_tempdir(tempdir.path()); - resolution1.compute_through_bidegree(Bidegree::s_t(6, 6)); -} - -#[test] -fn test_tempdir_unlock() { - let tempdir = tempfile::TempDir::new().unwrap(); - let resolution1 = - construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); - resolution1.compute_through_bidegree(Bidegree::s_t(5, 5)); - - lock_tempdir(tempdir.path()); - unlock_tempdir(tempdir.path()); - resolution1.compute_through_bidegree(Bidegree::s_t(6, 6)); -} - #[test] fn test_save_load() { let tempdir = tempfile::TempDir::new().unwrap(); @@ -74,19 +18,12 @@ fn test_save_load() { resolution1.compute_through_bidegree(Bidegree::s_t(6, 10)); resolution1.should_save = false; - let mut resolution2 = + let resolution2 = construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); - // Check that we are not writing anything new. - lock_tempdir(tempdir.path()); resolution2.compute_through_bidegree(Bidegree::s_t(10, 6)); resolution2.compute_through_bidegree(Bidegree::s_t(6, 10)); - resolution2.should_save = false; - - resolution1.compute_through_bidegree(Bidegree::s_t(20, 20)); - resolution2.compute_through_bidegree(Bidegree::s_t(20, 20)); - assert_eq!( resolution1.graded_dimension_string(), resolution2.graded_dimension_string() @@ -96,20 +33,20 @@ fn test_save_load() { resolution1.differential(5).quasi_inverse(7), resolution2.differential(5).quasi_inverse(7) ); - unlock_tempdir(tempdir.path()); } +/// Resolving once with the Adem algebra and then attempting to reuse the same save dir with the +/// Milnor algebra must fail loudly via the `bind_to_algebra` check, not silently mix data from +/// the two bases. Resurrected from the pre-zarr `wrong_algebra` test in master. #[test] -#[should_panic(expected = "Invalid header: algebra was 0x20000 but expected 0x28000")] -fn wrong_algebra() { +#[should_panic(expected = "different algebra")] +fn test_wrong_algebra() { let tempdir = tempfile::TempDir::new().unwrap(); let resolution1 = construct_standard::("S_2@adem", Some(tempdir.path().into())).unwrap(); resolution1.compute_through_bidegree(Bidegree::s_t(2, 2)); - let resolution2 = - construct_standard::("S_2@milnor", Some(tempdir.path().into())).unwrap(); - resolution2.compute_through_bidegree(Bidegree::s_t(2, 2)); + construct_standard::("S_2@milnor", Some(tempdir.path().into())).unwrap(); } #[test] @@ -123,7 +60,6 @@ fn test_save_load_stem() { let resolution2 = construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); - lock_tempdir(tempdir.path()); resolution2.compute_through_stem(Bidegree::n_s(10, 10)); @@ -136,7 +72,6 @@ fn test_save_load_stem() { resolution1.differential(5).quasi_inverse(7), resolution2.differential(5).quasi_inverse(7) ); - unlock_tempdir(tempdir.path()); } #[test] @@ -149,56 +84,15 @@ fn test_save_load_resume() { let resolution2 = construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); - lock_tempdir(tempdir.path()); resolution2.compute_through_stem(Bidegree::n_s(14, 8)); - unlock_tempdir(tempdir.path()); resolution1.compute_through_stem(Bidegree::n_s(19, 5)); - lock_tempdir(tempdir.path()); resolution2.compute_through_stem(Bidegree::n_s(19, 5)); assert_eq!( resolution1.graded_dimension_string(), resolution2.graded_dimension_string() ); - unlock_tempdir(tempdir.path()); -} - -#[test] -fn test_save_load_split() { - let tempdir_read = tempfile::TempDir::new().unwrap(); - let tempdir_write = tempfile::TempDir::new().unwrap(); - - let resolution = construct_standard::( - "S_2", - SaveDirectory::Combined(tempdir_read.path().into()), - ) - .unwrap(); - resolution.compute_through_stem(Bidegree::n_s(14, 8)); - lock_tempdir(tempdir_read.path()); - - let resolution = construct_standard::( - "S_2", - SaveDirectory::Split { - read: tempdir_read.path().into(), - write: tempdir_write.path().into(), - }, - ) - .unwrap(); - resolution.compute_through_stem(Bidegree::n_s(14, 8)); - - let contains_only_dirs = |p: &Path| { - p.read_dir().unwrap().all(|dir| { - let dir = dir.unwrap(); - dir.file_type().unwrap().is_dir() && dir.path().read_dir().unwrap().next().is_none() - }) - }; - - assert!(contains_only_dirs(tempdir_write.path())); - - resolution.compute_through_stem(Bidegree::n_s(19, 5)); - - assert!(!contains_only_dirs(tempdir_write.path())); } #[test] @@ -227,24 +121,8 @@ fn test_load_secondary() { lift1.initialize_homotopies(); lift1.compute_composites(); lift1.compute_intermediates(); - - let mut dir = tempdir.path().to_owned(); - let mut is_empty = |d| { - dir.push(d); - let result = dir.read_dir().unwrap().next().is_none(); - dir.pop(); - result - }; - - // Check that intermediates is non-empty - assert!(!is_empty("secondary_intermediates")); - lift1.compute_homotopies(); - assert!(is_empty("secondary_intermediates")); - assert!(!is_empty("secondary_homotopys")); - assert!(!is_empty("secondary_composites")); - // Load the resolution and extend further let mut resolution2 = construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); @@ -256,9 +134,6 @@ fn test_load_secondary() { lift2.compute_composites(); lift2.compute_homotopies(); - // Check that all intermediates are consumed - assert!(is_empty("secondary_intermediates")); - // Check that we have correct result assert_eq!(lift2.homotopy(3).homotopies.hom_k(16), vec![vec![1]]); @@ -275,32 +150,21 @@ fn test_load_secondary() { } #[test] -#[should_panic(expected = "Invalid file checksum")] -fn test_checksum() { - use std::{ - fs::OpenOptions, - io::{Seek, SeekFrom, Write}, - }; - +fn test_zarr_store_exists() { let tempdir = tempfile::TempDir::new().unwrap(); construct_standard::("S_2", Some(tempdir.path().into())) .unwrap() - .compute_through_bidegree(Bidegree::s_t(2, 2)); - - let mut path = tempdir.path().to_owned(); - path.push("differentials/2_2_differential"); - - let mut file = OpenOptions::new() - .read(true) - .write(true) - .open(path) - .unwrap(); + .compute_through_bidegree(Bidegree::s_t(3, 3)); - file.seek(SeekFrom::Start(41)).unwrap(); - file.write_all(&[1]).unwrap(); - - construct_standard::("S_2", Some(tempdir.path().into())) - .unwrap() - .compute_through_bidegree(Bidegree::s_t(2, 2)); + // Verify data was stored in zarr format + let store_path = tempdir.path(); + assert!( + store_path.join("zarr.json").exists(), + "zarr root group missing" + ); + assert!( + store_path.join("differential/zarr.json").exists(), + "shard-tier differential array missing" + ); } From b9d5ea340310bd3b1f8ae403312a911f87fa9a52 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 01:53:27 +0000 Subject: [PATCH 02/13] save: cache opened zarr arrays and lock per-shard, not per-kind Two hot-path improvements to ZarrSaveStore that remove overhead the rebase inherited, with no change to the on-disk format or public API. 1. Cache opened Array handles (`arrays: DashMap>`). Previously every read/write/delete called `zarrs::array::Array::open`, which re-reads and re-parses the array's `zarr.json` through the storage backend on each call. The handle holds no chunk data and its methods take `&self`, so a cached `Arc` is safe to share for concurrent reads and (shard-serialized) writes. Now the metadata is parsed at most once per kind per store. Read/delete of a not-yet-created kind still returns None/Ok as before. 2. Key the write lock by `(kind, shard coords)` instead of by kind alone. The zarrs read-modify-write contract only requires serializing writes that share a shard; the old per-kind lock needlessly serialized every write of a kind across the whole store. Writes to different shards touch disjoint chunk files, so per-shard locking restores the cross-shard parallelism a parallel resolution depends on while still honouring the contract. Kept `with_concurrent_target(1)` so the sharding codec stays sequential and the rayon-join deadlock the original guarded against cannot occur. Verified on S_2 through-stem (n=70, s=40), release + concurrent, 4 cores: best-of-3 wall time 15.1s -> 13.0s (~14% faster) with identical Ext output. Save+resume roundtrip re-checked for both the standard and nassau backends; all lib and save_load_resolution tests pass. --- ext/src/save.rs | 106 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 23 deletions(-) diff --git a/ext/src/save.rs b/ext/src/save.rs index 8d635249d0..b97d1d0c96 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -192,17 +192,30 @@ pub struct ZarrSaveStore { /// /// Used to skip the `meta_path` check on subsequent writes. created: DashSet, - /// Per-kind write lock. + /// Cache of opened shard-tier [`zarrs::array::Array`] handles, one per [`SaveKind`]. + /// + /// Opening an array parses its `zarr.json` metadata through the storage backend (a file read + /// + `serde_json` parse on the filesystem store). Every `read`/`write`/`delete` targets the + /// same handful of per-kind arrays, so we open each at most once and reuse the handle. The + /// `Array` holds no chunk data — chunks are fetched/stored through the shared store on each + /// call — and its methods take `&self`, so a cached `Arc` is safe to share across + /// threads for concurrent reads and (shard-serialized) writes. + arrays: DashMap>, + /// Per-shard write lock. /// /// Since zarrs 0.14, `Array::store_array_subset` is documented as requiring caller-side - /// synchronization for "regions sharing any chunks" — the codec does a read-modify-write on - /// the entire shard internally, so concurrent calls touching different inner chunks of the - /// same shard race and lose writes. We serialize per kind, which is coarser than required - /// (per shard would suffice) but simpler and entirely sufficient for our workload, since - /// cross-kind parallelism dominates. - write_locks: DashMap>>, + /// synchronization for "regions sharing any chunks" — the sharding codec does a + /// read-modify-write on the entire shard internally, so concurrent calls touching different + /// inner chunks of the *same shard* race and lose writes. Writes to *different* shards touch + /// disjoint chunk files and are independent, so we key the lock by `(kind, shard coords)` + /// rather than by kind alone. This preserves the cross-shard write parallelism a parallel + /// resolution relies on while still honouring the zarrs contract. + write_locks: DashMap<(SaveKind, [u64; 3]), Arc>>, } +/// Alias for the concrete opened-array type shared across the store and its streaming readers. +type ShardArray = zarrs::array::Array; + // See the doc comment on `ZarrSaveStore::store` for why this is sound on wasm. On native the // trait object is already `Send + Sync`, so the cfg gate avoids a redundant-impl warning. #[cfg(target_arch = "wasm32")] @@ -249,6 +262,7 @@ impl ZarrSaveStore { store, group: String::new(), created: DashSet::new(), + arrays: DashMap::new(), write_locks: DashMap::new(), }) } @@ -328,6 +342,7 @@ impl ZarrSaveStore { store: self.store.clone(), group, created: DashSet::new(), + arrays: DashMap::new(), write_locks: DashMap::new(), }) } @@ -385,13 +400,30 @@ impl ZarrSaveStore { out } - /// Get-or-create the per-kind write lock for this store. + /// Shard-tier chunk coordinates (== shard coordinates, since the chunk shape *is* the shard + /// shape) covering `zarr_coords`. 2D kinds get a `0` in the third slot. + /// + /// A single-element `store_array_subset` at `zarr_coords` touches exactly this one shard, so + /// serializing on it is both necessary and sufficient for the zarrs read-modify-write + /// contract. + fn shard_coords(zarr_coords: &[u64]) -> [u64; 3] { + let mut out = [0u64; 3]; + out[0] = zarr_coords[0] / SHARD_N; + out[1] = zarr_coords[1] / SHARD_S; + if zarr_coords.len() > 2 { + out[2] = zarr_coords[2] / SHARD_IDX; + } + out + } + + /// Get-or-create the write lock guarding the shard that holds `zarr_coords` for `kind`. /// /// See the comment on the `write_locks` field for why this exists. - fn write_lock(&self, kind: SaveKind) -> Arc> { + fn write_lock(&self, kind: SaveKind, zarr_coords: &[u64]) -> Arc> { + let key = (kind, Self::shard_coords(zarr_coords)); Arc::clone( self.write_locks - .entry(kind) + .entry(key) .or_insert_with(|| Arc::new(Mutex::new(()))) .value(), ) @@ -429,6 +461,31 @@ impl ZarrSaveStore { Ok(()) } + /// Return the cached (or freshly opened) shard-tier array handle for `kind`. + /// + /// With `create`, the array is created if absent (used by writes). Without, a missing array + /// yields `None` (used by reads, before anything of that kind has been written). The opened + /// handle is memoized in `self.arrays`, so the `zarr.json` metadata is parsed at most once + /// per kind per store. + fn shard_array(&self, kind: SaveKind, create: bool) -> anyhow::Result>> { + if let Some(arr) = self.arrays.get(&kind) { + return Ok(Some(Arc::clone(arr.value()))); + } + if create { + self.ensure_shard_array(kind)?; + } + match zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind)) { + Ok(arr) => { + let arr = Arc::new(arr); + // If another thread opened it first, keep the existing handle; they're equivalent. + let cached = Arc::clone(self.arrays.entry(kind).or_insert(arr).value()); + Ok(Some(cached)) + } + Err(_) if !create => Ok(None), + Err(e) => Err(zarr_err(e)), + } + } + /// Lazily create `qi/` and `qi/n{n}_s{s}/` groups under this group prefix. fn ensure_qi_bidegree(&self, b: Bidegree) -> anyhow::Result<()> { let qi_root = self.group_fs_path().join("qi"); @@ -471,11 +528,12 @@ impl ZarrSaveStore { "write() is only for sharded kinds, got {:?}", kind ); - self.ensure_shard_array(kind)?; - let lock = self.write_lock(kind); - let _guard = lock.lock().unwrap(); - let arr = zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind))?; + let arr = self + .shard_array(kind, true)? + .expect("shard_array(create=true) never returns None"); let zarr_coords = Self::shard_zarr_coords(location.save_coords()); + let lock = self.write_lock(kind, &zarr_coords); + let _guard = lock.lock().unwrap(); let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; // Force sequential codec execution. Holding our std::sync::Mutex across // `store_array_subset` is unsafe with rayon, because zarrs's sharding codec uses rayon @@ -505,10 +563,9 @@ impl ZarrSaveStore { "read() is only for sharded kinds, got {:?}", kind ); - let arr = match zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind)) - { - Ok(arr) => arr, - Err(_) => return Ok(None), + let arr = match self.shard_array(kind, false)? { + Some(arr) => arr, + None => return Ok(None), }; let zarr_coords = Self::shard_zarr_coords(location.save_coords()); let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; @@ -540,12 +597,15 @@ impl ZarrSaveStore { "delete() is only for sharded kinds, got {:?}", kind ); - // Overwrite with fill value (empty vec). The array must already exist for delete to be - // meaningful. Same locking + sequential codec story as `write`. - let lock = self.write_lock(kind); - let _guard = lock.lock().unwrap(); - let arr = zarrs::array::Array::open(self.store.clone(), &self.shard_array_path(kind))?; + // Overwrite with fill value (empty vec). A missing array means there is nothing to + // delete, so treat that as success. Same locking + sequential codec story as `write`. + let arr = match self.shard_array(kind, false)? { + Some(arr) => arr, + None => return Ok(()), + }; let zarr_coords = Self::shard_zarr_coords(location.save_coords()); + let lock = self.write_lock(kind, &zarr_coords); + let _guard = lock.lock().unwrap(); let subset = ArraySubset::new_with_start_shape(zarr_coords, vec![1; N])?; arr.store_array_subset_opt( &subset, From 49797146a2ccf3ef05019c164c7f4d17676e2f53 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 02:28:38 +0000 Subject: [PATCH 03/13] save: default stream-tier zstd to level 3, override via EXT_SAVE_ZSTD_LEVEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebased branch hardcoded zstd level 19 for the stream tier (res_qi, nassau_qi), which is on the hot write path — every differential quasi-inverse is compressed there. Benchmarked against master's uncompressed old format (S_2 through-stem n=70 s=40, release + concurrent, 4 cores, best of 3): master (old, uncompressed): 2.55s write, 51M on disk (real bytes) zarr, zstd level 19: 13.0s write, 23M zarr, zstd level 3: 3.65s write, 24M Level 19 was ~5x slower than master for a 4% smaller store than level 3 on this dense, high-entropy data. Level 3 (zstd's own default) keeps nearly all the compression — still less than half of master's on-disk size — at a fraction of the write cost, cutting the regression from ~5x to ~1.4x. The read/resume path is unchanged and already at parity with master (~0.10s). Compression still matters for very large runs, so the level is read once from the EXT_SAVE_ZSTD_LEVEL environment variable (clamped to zstd's [1, 22]; unparseable values warn and fall back to the default). Set e.g. EXT_SAVE_ZSTD_LEVEL=19 when the on-disk footprint, not save time, is the binding constraint. --- ext/src/save.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/ext/src/save.rs b/ext/src/save.rs index b97d1d0c96..a12e0efd81 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -48,6 +48,11 @@ //! Group attributes include a `finished` flag, which is the source of truth — readers treat the //! data as missing if the writer was dropped before calling `finish()`. //! +//! Stream-tier arrays are zstd-compressed. The level defaults to `3` (zstd's own default: most of +//! the ratio at a fraction of the write cost of the maximum level) and can be overridden with the +//! `EXT_SAVE_ZSTD_LEVEL` environment variable for very large runs where the on-disk footprint +//! matters more than save time. +//! //! Subgroups share the same underlying [`FilesystemStore`] via `Arc` clone; only the `group` //! prefix differs. Shard arrays are created lazily on first write so that subgroups don't //! populate kinds they never use. @@ -119,9 +124,52 @@ const SHARD_S: u64 = 8; /// Shard shape in the idx dimension. const SHARD_IDX: u64 = 8; -/// zstd compression level for the stream tier. +/// Default stream-tier zstd level when [`ZSTD_LEVEL_ENV`] is unset. +/// +/// Level 3 is zstd's own default and sits at the knee of the ratio/speed curve. The stream tier +/// (`res_qi`, `nassau_qi`) is on the hot write path — every differential quasi-inverse is +/// compressed here — so a high level makes saving several times slower for a small extra ratio +/// on this already-dense data. Large runs that want to trade write time for maximum on-disk +/// compression can raise the level via the environment variable below. +#[cfg(not(target_arch = "wasm32"))] +const DEFAULT_ZSTD_LEVEL: i32 = 3; + +/// Environment variable that overrides the stream-tier zstd level. +/// +/// For very large resolutions (where the on-disk footprint, not save time, is the binding +/// constraint) set e.g. `EXT_SAVE_ZSTD_LEVEL=19` for near-maximum compression. The value is read +/// once, parsed as an integer, and clamped to zstd's valid `[1, 22]` range; anything unparseable +/// falls back to [`DEFAULT_ZSTD_LEVEL`] with a warning. +#[cfg(not(target_arch = "wasm32"))] +const ZSTD_LEVEL_ENV: &str = "EXT_SAVE_ZSTD_LEVEL"; + +/// Resolve the stream-tier zstd level, reading [`ZSTD_LEVEL_ENV`] at most once. #[cfg(not(target_arch = "wasm32"))] -const ZSTD_LEVEL: i32 = 19; +fn zstd_level() -> i32 { + use std::sync::LazyLock; + static LEVEL: LazyLock = LazyLock::new(|| match std::env::var(ZSTD_LEVEL_ENV) { + Ok(s) => match s.trim().parse::() { + Ok(v) => { + let clamped = v.clamp(1, 22); + if clamped != v { + tracing::warn!( + "{ZSTD_LEVEL_ENV}={v} is outside zstd's [1, 22] range; using {clamped}" + ); + } + clamped + } + Err(_) => { + tracing::warn!( + "{ZSTD_LEVEL_ENV}={s:?} is not a valid integer; using default level \ + {DEFAULT_ZSTD_LEVEL}" + ); + DEFAULT_ZSTD_LEVEL + } + }, + Err(_) => DEFAULT_ZSTD_LEVEL, + }); + *LEVEL +} /// Convert a zarrs error into `anyhow::Error`. /// @@ -144,7 +192,7 @@ fn stream_tier_codecs() -> Vec> { #[cfg(not(target_arch = "wasm32"))] { vec![ - Arc::new(ZstdCodec::new(ZSTD_LEVEL, false)), + Arc::new(ZstdCodec::new(zstd_level(), false)), Arc::new(Crc32cCodec::new()), ] } From 572fb261f9b7b69e4b0cc80ac8303fa631797862 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 02:41:11 +0000 Subject: [PATCH 04/13] save: reword arrays field doc to satisfy clippy doc_lazy_continuation A line beginning with "+ serde_json parse" was parsed as a markdown "+" list marker, so clippy (-D warnings in CI's lint job) flagged the following lines as list items without indentation. Reword to "and a serde_json parse". No code change. --- ext/src/save.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/src/save.rs b/ext/src/save.rs index a12e0efd81..ca637657f1 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -243,8 +243,8 @@ pub struct ZarrSaveStore { /// Cache of opened shard-tier [`zarrs::array::Array`] handles, one per [`SaveKind`]. /// /// Opening an array parses its `zarr.json` metadata through the storage backend (a file read - /// + `serde_json` parse on the filesystem store). Every `read`/`write`/`delete` targets the - /// same handful of per-kind arrays, so we open each at most once and reuse the handle. The + /// and a `serde_json` parse on the filesystem store). Every `read`/`write`/`delete` targets + /// the same handful of per-kind arrays, so we open each at most once and reuse the handle. The /// `Array` holds no chunk data — chunks are fetched/stored through the shared store on each /// call — and its methods take `&self`, so a cached `Arc` is safe to share across /// threads for concurrent reads and (shard-serialized) writes. From 9632e8981704f00ad75460ebe968111db47d82cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:01:20 +0000 Subject: [PATCH 05/13] save: harden a few fallible paths flagged in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the low-risk, verified subset of CodeRabbit's review on #260: - ResQiReader::apply / into_quasi_inverse: replace assert!(got, ...) with anyhow::ensure!(...). Both are already anyhow::Result-returning, so a truncated stream now propagates an error instead of aborting the caller. - NassauQiReader::parse: reject odd-length Signature payloads before chunks_exact(2) silently drops a trailing byte. - Stream-kind guards in write/read/exists/delete: promote debug_assert! to assert! so misusing a stream kind (ResQi/NassauQi) on the shard-tier API is caught in release too, not just debug. Deliberately not applied from the same review: - "Also assert target_res_dimension in the differential load path" — this is a false positive: test_load_smaller legitimately resumes into a smaller target range, so the read-time resolution dimension differs from the saved one (the rows carry their own widths). The original code asserts only target_cc_dimension for exactly this reason. - Prime check in bind_to_algebra — redundant: magic() already encodes the prime (p << 16), so a prime mismatch already fails the existing magic check. --- ext/src/save.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ext/src/save.rs b/ext/src/save.rs index ca637657f1..518a6bad93 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -571,7 +571,7 @@ impl ZarrSaveStore { location: impl SaveCoords, data: &[u8], ) -> anyhow::Result<()> { - debug_assert!( + assert!( !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), "write() is only for sharded kinds, got {:?}", kind @@ -606,7 +606,7 @@ impl ZarrSaveStore { kind: SaveKind, location: impl SaveCoords, ) -> anyhow::Result>> { - debug_assert!( + assert!( !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), "read() is only for sharded kinds, got {:?}", kind @@ -626,7 +626,7 @@ impl ZarrSaveStore { /// Check if a sharded payload exists. pub fn exists(&self, kind: SaveKind, location: impl SaveCoords) -> bool { - debug_assert!( + assert!( !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), "exists() is only for sharded kinds, got {:?}", kind @@ -640,7 +640,7 @@ impl ZarrSaveStore { kind: SaveKind, location: impl SaveCoords, ) -> anyhow::Result<()> { - debug_assert!( + assert!( !matches!(kind, SaveKind::ResQi | SaveKind::NassauQi), "delete() is only for sharded kinds, got {:?}", kind @@ -1045,7 +1045,7 @@ impl ResQiReader { continue; } let got = self.next_row(&mut row)?; - assert!(got, "ResQi truncated: expected row for pivot {i}"); + anyhow::ensure!(got, "ResQi truncated: expected row for pivot {i}"); for (input, result) in inputs.iter().zip_eq(results.iter_mut()) { result.into().add(row.as_slice(), input.into().entry(i)); } @@ -1061,7 +1061,7 @@ impl ResQiReader { for _ in 0..self.image_dim { let mut row = FpVector::new(self.p, self.source_dim); let got = self.next_row(&mut row)?; - assert!(got, "ResQi truncated while materializing"); + anyhow::ensure!(got, "ResQi truncated while materializing"); rows.push(row); } let preimage = Matrix::from_rows(self.p, rows, self.source_dim); @@ -1226,6 +1226,11 @@ impl NassauQiReader { match code { NASSAU_CODE_SIGNATURE => { let payload = &bytes[8..]; + anyhow::ensure!( + payload.len().is_multiple_of(2), + "NassauQi signature payload has odd length {}", + payload.len() + ); let sig: Vec = payload .chunks_exact(2) .map(|c| u16::from_le_bytes([c[0], c[1]])) From 6f73a0097276d3fd420f8412958a84f03474d02b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:40:47 +0000 Subject: [PATCH 06/13] save: make SaveDirectory conversion fallible (TryFrom, not panicking From) Addresses the remaining actionable review finding on #260. Store creation can fail (bad path, permissions, unreadable existing metadata), but the old From> swallowed that with .expect(), so new_with_save's anyhow::Result could never surface it. Replace the impl with TryFrom> (Error = anyhow::Error) and thread the fallible conversion through the entry points that take a save dir: Resolution / nassau::Resolution::new_with_save and utils::construct{,_nassau, _standard} now take `impl TryInto` and `?`-propagate. Every existing call site passes Option, so this is source-compatible; a bad save path now returns an Err instead of panicking. Not applied from the same review (left for the author's call): name sanitization for subgroup paths, dimension-framing headers for chain-map/homotopy payloads, and per-complex store fingerprinting. --- ext/src/nassau.rs | 4 ++-- ext/src/resolution.rs | 4 ++-- ext/src/save.rs | 14 ++++++++++---- ext/src/utils.rs | 6 +++--- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index 95e2ea0784..26a397164b 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -338,9 +338,9 @@ impl> Resolution { pub fn new_with_save( module: Arc, - save_dir: impl Into, + save_dir: impl TryInto, ) -> anyhow::Result { - let save_dir = save_dir.into(); + let save_dir = save_dir.try_into()?; let max_degree = module .max_degree() .ok_or_else(|| anyhow!("Nassau's algorithm requires bounded module"))?; diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index 6e5103b529..49f4e5011e 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -133,9 +133,9 @@ where pub fn new_with_save( complex: Arc, - save_dir: impl Into, + save_dir: impl TryInto, ) -> anyhow::Result { - let save_dir = save_dir.into(); + let save_dir = save_dir.try_into()?; let algebra = complex.algebra(); if let Some(store) = save_dir.store() { store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; diff --git a/ext/src/save.rs b/ext/src/save.rs index 518a6bad93..3b79be8e7d 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -1403,11 +1403,17 @@ impl SaveDirectory { } } -impl From> for SaveDirectory { - fn from(x: Option) -> Self { +impl TryFrom> for SaveDirectory { + type Error = anyhow::Error; + + /// `None` yields [`SaveDirectory::None`]; `Some(path)` opens (or creates) the zarr store at + /// that path. Store creation is fallible (bad path, permissions, corrupt existing metadata), + /// so this is a `TryFrom` rather than a panicking `From` — the error propagates through the + /// `new_with_save` / `construct*` call chain instead of aborting. + fn try_from(x: Option) -> anyhow::Result { match x { - None => Self::None, - Some(p) => Self::Store(ZarrSaveStore::create(p).expect("Failed to create zarr store")), + None => Ok(Self::None), + Some(p) => Ok(Self::Store(ZarrSaveStore::create(p)?)), } } } diff --git a/ext/src/utils.rs b/ext/src/utils.rs index ea1c046eea..595e8cd369 100644 --- a/ext/src/utils.rs +++ b/ext/src/utils.rs @@ -140,7 +140,7 @@ impl> TryFrom<(Value, T)> for Config { /// the `nassau` feature is enabled. pub fn construct( module_spec: T, - save_dir: impl Into, + save_dir: impl TryInto, ) -> anyhow::Result where anyhow::Error: From, @@ -160,7 +160,7 @@ where /// See [`construct`] pub fn construct_nassau( module_spec: T, - save_dir: impl Into, + save_dir: impl TryInto, ) -> anyhow::Result>> where anyhow::Error: From, @@ -200,7 +200,7 @@ where /// See [`construct`] pub fn construct_standard( module_spec: T, - save_dir: impl Into, + save_dir: impl TryInto, ) -> anyhow::Result> where anyhow::Error: From, From d1e9d949cc89d2f4fe655e250d993b3c7c441a6e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:48:03 +0000 Subject: [PATCH 07/13] save: use as_chunks::<2> for signature parse (nightly clippy lint) CI's lint job runs clippy on nightly, whose chunks_exact_to_as_chunks lint (not yet in the stable clippy I checked locally) rejects chunks_exact(2) with a constant size. Switch the NassauQi signature decode to as_chunks::<2>(); the even-length check just above guarantees an empty remainder, so this is equivalent. as_chunks is stable since 1.88, well under the toolchains CI tests. --- ext/src/save.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ext/src/save.rs b/ext/src/save.rs index 3b79be8e7d..0831ea8f4e 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -1231,9 +1231,14 @@ impl NassauQiReader { "NassauQi signature payload has odd length {}", payload.len() ); + // Even length checked above, so `as_chunks` leaves no remainder. (Using + // `as_chunks` rather than `chunks_exact(2)` also satisfies clippy's + // `chunks_exact_to_as_chunks` lint on nightly.) let sig: Vec = payload - .chunks_exact(2) - .map(|c| u16::from_le_bytes([c[0], c[1]])) + .as_chunks::<2>() + .0 + .iter() + .map(|&[a, b]| u16::from_le_bytes([a, b])) .collect(); Ok(NassauCommand::Signature(sig)) } From faf61b1777828118a692179d41172713f775345b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 04:17:36 +0000 Subject: [PATCH 08/13] save: relax new_with_save/construct bound to any TryInto Follow-up to the TryFrom change: the bound `TryInto` was stricter than the original `Into` because the reflexive `TryInto` for `SaveDirectory` has `Error = Infallible`, so a caller passing an already-built `SaveDirectory` was rejected. Widen the bound to `Error: Into` (accepts both `Option` with `anyhow::Error` and `SaveDirectory` with `Infallible`) and convert via `.map_err(Into::into)`, restoring the original API surface while keeping the fallible path for store creation. --- ext/src/nassau.rs | 4 ++-- ext/src/resolution.rs | 4 ++-- ext/src/utils.rs | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index 26a397164b..becc65b86f 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -338,9 +338,9 @@ impl> Resolution { pub fn new_with_save( module: Arc, - save_dir: impl TryInto, + save_dir: impl TryInto>, ) -> anyhow::Result { - let save_dir = save_dir.try_into()?; + let save_dir = save_dir.try_into().map_err(Into::into)?; let max_degree = module .max_degree() .ok_or_else(|| anyhow!("Nassau's algorithm requires bounded module"))?; diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index 49f4e5011e..055a1e8820 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -133,9 +133,9 @@ where pub fn new_with_save( complex: Arc, - save_dir: impl TryInto, + save_dir: impl TryInto>, ) -> anyhow::Result { - let save_dir = save_dir.try_into()?; + let save_dir = save_dir.try_into().map_err(Into::into)?; let algebra = complex.algebra(); if let Some(store) = save_dir.store() { store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; diff --git a/ext/src/utils.rs b/ext/src/utils.rs index 595e8cd369..539638c158 100644 --- a/ext/src/utils.rs +++ b/ext/src/utils.rs @@ -140,7 +140,7 @@ impl> TryFrom<(Value, T)> for Config { /// the `nassau` feature is enabled. pub fn construct( module_spec: T, - save_dir: impl TryInto, + save_dir: impl TryInto>, ) -> anyhow::Result where anyhow::Error: From, @@ -160,7 +160,7 @@ where /// See [`construct`] pub fn construct_nassau( module_spec: T, - save_dir: impl TryInto, + save_dir: impl TryInto>, ) -> anyhow::Result>> where anyhow::Error: From, @@ -200,7 +200,7 @@ where /// See [`construct`] pub fn construct_standard( module_spec: T, - save_dir: impl TryInto, + save_dir: impl TryInto>, ) -> anyhow::Result> where anyhow::Error: From, From 189e2dce5e17d7e85b1ebe9bc84f167c211f0219 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 22:39:46 +0000 Subject: [PATCH 09/13] Address review comments from @hoodmane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - chain_homotopy.rs: restore the original ChainHomotopy field order (homotopies before save_dir). The rework had swapped them for no functional reason (field order here doesn't affect drop semantics or the constructor); revert to minimize churn. - save.rs: the wasm rationale said the filesystem store "doesn't compile for WASM", but that's specific to wasm32-unknown-unknown — wasm32-unknown-emscripten is `unix` and unaffected. Reword the module doc and the parallel inline comment to name the target precisely. --- ext/src/chain_complex/chain_homotopy.rs | 4 ++-- ext/src/save.rs | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/ext/src/chain_complex/chain_homotopy.rs b/ext/src/chain_complex/chain_homotopy.rs index 13f269c994..47c431a36e 100644 --- a/ext/src/chain_complex/chain_homotopy.rs +++ b/ext/src/chain_complex/chain_homotopy.rs @@ -29,13 +29,13 @@ pub struct ChainHomotopy< left: Arc>, right: Arc>, lock: Mutex<()>, + /// Homotopies, indexed by the filtration of the target of f - g. + homotopies: OnceBiVec>>, /// Save directory for this homotopy and any secondary lift built from it. /// Set to a `homotopies/{left}__{right}` subgroup of the source's save_dir /// when both `left` and `right` are named, `None` otherwise so that /// homotopies between anonymous homs don't pollute the store. save_dir: SaveDirectory, - /// Homotopies, indexed by the filtration of the target of f - g. - homotopies: OnceBiVec>>, } impl< diff --git a/ext/src/save.rs b/ext/src/save.rs index 0831ea8f4e..143bf79201 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -3,10 +3,11 @@ //! On native targets the store is backed by a real zarr v3 store on the local filesystem via //! [`zarrs::filesystem::FilesystemStore`]. On `wasm32-unknown-unknown` we swap in //! [`zarrs::storage::store::MemoryStore`] instead — `zarrs_filesystem` transitively pulls in -//! `positioned-io::RandomAccessFile`, which is gated to `cfg(any(windows, unix))` and doesn't -//! compile for WASM. The WASM frontend has no filesystem to persist to anyway, so the memory -//! store just acts as a no-op sink that's dropped at session end; the same code paths -//! exercise it on both targets. +//! `positioned-io::RandomAccessFile`, which is gated to `cfg(any(windows, unix))` and so doesn't +//! compile for `wasm32-unknown-unknown` (which is neither — note `wasm32-unknown-emscripten` *is* +//! `unix`, so this only concerns the `unknown` OS). That target is the only wasm consumer (the web +//! frontend), and it has no filesystem to persist to anyway, so the memory store just acts as a +//! no-op sink that's dropped at session end; the same code paths exercise it on both targets. //! //! # Layout //! @@ -83,10 +84,10 @@ use sseq::coordinates::{Bidegree, BidegreeGenerator}; #[cfg(not(target_arch = "wasm32"))] use zarrs::array::codec::ZstdCodec; // The concrete backing store depends on the target: native uses a `FilesystemStore` for real -// on-disk persistence; WASM uses an in-memory `MemoryStore` so `zarrs_filesystem` (which pulls -// in `positioned-io::RandomAccessFile`, gated to windows/unix) doesn't need to compile. The -// WASM frontend has no filesystem to persist to anyway, so the memory store just serves as a -// no-op sink and is dropped at session end. +// on-disk persistence; `wasm32-unknown-unknown` uses an in-memory `MemoryStore` so +// `zarrs_filesystem` (which pulls in `positioned-io::RandomAccessFile`, gated to windows/unix) +// doesn't need to compile. The web frontend has no filesystem to persist to anyway, so the +// memory store just serves as a no-op sink and is dropped at session end. #[cfg(not(target_arch = "wasm32"))] use zarrs::filesystem::FilesystemStore; #[cfg(target_arch = "wasm32")] From 76caf4ddbeda991f91d6ca60ddebe55ca5b28fe6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 05:07:54 +0000 Subject: [PATCH 10/13] save: gate backing store on any(windows, unix) via a platform module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses @hoodmane's review: gate on what the dependencies actually require rather than target_arch = "wasm32", and stop repeating the cfg everywhere. The zarr `filesystem` feature pulls in `positioned-io::RandomAccessFile`, which is itself `cfg(any(windows, unix))`; the `zstd` feature's `zstd-sys` C build needs the same POSIX platform. So `any(windows, unix)` — not `target_arch = "wasm32"` — is the authoritative condition: `wasm32-unknown- emscripten` is `unix` and keeps the filesystem+zstd path, while `wasm32-unknown-unknown` (the web frontend) and `wasm32-wasi` fall back to the in-memory store with CRC32C-only codecs. - Cargo.toml: select the `zarrs` feature set on `cfg(any(windows, unix))` / `cfg(not(any(windows, unix)))`. - save.rs: collapse the ~14 scattered `#[cfg(target_arch = "wasm32")]` sites into two `#[cfg]`-gated `platform` submodules exposing `open_store`, `root_group_missing`, and `stream_tier_codecs` (plus the zstd-level machinery and the wasm `Send`/`Sync` impls). The rest of the file is now cfg-free. Behaviour on the two built targets (native, wasm32-unknown-unknown) is unchanged; only the previously-untargeted emscripten/wasi cases move. Verified: native check + nightly clippy/fmt, wasm32-unknown-unknown build of sseq_gui, and the save_load + lib test suites. --- ext/Cargo.toml | 18 ++-- ext/src/save.rs | 229 ++++++++++++++++++++++++++---------------------- 2 files changed, 136 insertions(+), 111 deletions(-) diff --git a/ext/Cargo.toml b/ext/Cargo.toml index bb1128e18a..0b71a7c531 100644 --- a/ext/Cargo.toml +++ b/ext/Cargo.toml @@ -37,16 +37,18 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } zstd = { version = "0.13.3", optional = true } -# Zarrs is declared twice with target-specific feature sets. On native targets we enable the -# `filesystem` feature so the save system persists to disk; on `wasm32-unknown-unknown` we -# fall back to an in-memory store (see `src/save.rs`). The `filesystem` feature pulls in -# `positioned-io::RandomAccessFile`, which is gated to `cfg(any(windows, unix))` and doesn't -# compile on wasm, and the `zstd` feature pulls in `zstd-sys`, whose C build expects POSIX -# symbols like `qsort_r` that wasm's libc shim doesn't expose — so both are dropped on wasm. -[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +# Zarrs is declared twice with target-specific feature sets. Where there is a real OS +# (`cfg(any(windows, unix))`) we enable `filesystem` so the save system persists to disk; on +# every other target (in practice `wasm32-unknown-unknown`, the web frontend) we fall back to an +# in-memory store (see the `platform` modules in `src/save.rs`). The gate is `any(windows, unix)`, +# not `not(target_arch = "wasm32")`, because that's exactly the `cfg` that gates the `filesystem` +# feature's `positioned-io::RandomAccessFile`; the `zstd` feature's `zstd-sys` C build (which +# expects POSIX symbols like `qsort_r` that the bare-wasm libc shim doesn't expose) builds under +# the same condition. So `wasm32-unknown-emscripten` (which is `unix`) keeps both features. +[target.'cfg(any(windows, unix))'.dependencies] zarrs = { version = "0.23", default-features = false, features = ["filesystem", "sharding", "crc32c", "zstd"] } -[target.'cfg(target_arch = "wasm32")'.dependencies] +[target.'cfg(not(any(windows, unix)))'.dependencies] zarrs = { version = "0.23", default-features = false, features = ["sharding", "crc32c"] } [target.'cfg(unix)'.dependencies] diff --git a/ext/src/save.rs b/ext/src/save.rs index 143bf79201..120ffb779c 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -81,23 +81,130 @@ use fp::{ vector::{FpSlice, FpSliceMut, FpVector}, }; use sseq::coordinates::{Bidegree, BidegreeGenerator}; -#[cfg(not(target_arch = "wasm32"))] -use zarrs::array::codec::ZstdCodec; -// The concrete backing store depends on the target: native uses a `FilesystemStore` for real -// on-disk persistence; `wasm32-unknown-unknown` uses an in-memory `MemoryStore` so -// `zarrs_filesystem` (which pulls in `positioned-io::RandomAccessFile`, gated to windows/unix) -// doesn't need to compile. The web frontend has no filesystem to persist to anyway, so the -// memory store just serves as a no-op sink and is dropped at session end. -#[cfg(not(target_arch = "wasm32"))] -use zarrs::filesystem::FilesystemStore; -#[cfg(target_arch = "wasm32")] -use zarrs::storage::store::MemoryStore; use zarrs::{ array::{ArrayBuilder, ArraySubset, CodecOptions, codec::Crc32cCodec, data_type}, group::GroupBuilder, storage::{ReadableWritableListableStorage, ReadableWritableListableStorageTraits}, }; +// --- Platform backing-store selection --- +// +// The zarr `filesystem` and `zstd` features (which pull in `positioned-io::RandomAccessFile` and +// `zstd-sys`) only build where there is a real OS: `cfg(any(windows, unix))`. That — not +// `target_arch = "wasm32"` — is the authoritative condition, because it's exactly the `cfg` that +// gates `positioned-io::RandomAccessFile`. `wasm32-unknown-emscripten` *is* `unix` and would use +// the filesystem store; `wasm32-unknown-unknown` (the web frontend) and `wasm32-wasi` are neither +// `windows` nor `unix`, so they fall back to an in-memory store with CRC32C-only codecs. The +// Cargo.toml feature selection uses the same predicate, and all the target `cfg` lives in the two +// `platform` modules below so the rest of this file is platform-agnostic. + +#[cfg(any(windows, unix))] +mod platform { + use std::sync::LazyLock; + + use zarrs::{array::codec::ZstdCodec, filesystem::FilesystemStore}; + + use super::*; + + /// Open the on-disk zarr store rooted at `path`. + pub fn open_store(path: &Path) -> anyhow::Result { + Ok(Arc::new(FilesystemStore::new(path)?)) + } + + /// Whether the root group's `zarr.json` still needs to be written — i.e. it doesn't already + /// exist on disk. + pub fn root_group_missing(path: &Path) -> bool { + !path.join("zarr.json").exists() + } + + /// Stream-tier codec chain: zstd (see [`zstd_level`]) followed by CRC32C. + pub fn stream_tier_codecs() -> Vec> { + vec![ + Arc::new(ZstdCodec::new(zstd_level(), false)), + Arc::new(Crc32cCodec::new()), + ] + } + + /// Default stream-tier zstd level when [`ZSTD_LEVEL_ENV`] is unset. + /// + /// Level 3 is zstd's own default and sits at the knee of the ratio/speed curve. The stream + /// tier (`res_qi`, `nassau_qi`) is on the hot write path — every differential quasi-inverse is + /// compressed here — so a high level makes saving several times slower for a small extra ratio + /// on this already-dense data. Large runs that want to trade write time for maximum on-disk + /// compression can raise the level via the environment variable below. + const DEFAULT_ZSTD_LEVEL: i32 = 3; + + /// Environment variable that overrides the stream-tier zstd level. + /// + /// For very large resolutions (where the on-disk footprint, not save time, is the binding + /// constraint) set e.g. `EXT_SAVE_ZSTD_LEVEL=19` for near-maximum compression. The value is + /// read once, parsed as an integer, and clamped to zstd's valid `[1, 22]` range; anything + /// unparseable falls back to [`DEFAULT_ZSTD_LEVEL`] with a warning. + const ZSTD_LEVEL_ENV: &str = "EXT_SAVE_ZSTD_LEVEL"; + + /// Resolve the stream-tier zstd level, reading [`ZSTD_LEVEL_ENV`] at most once. + fn zstd_level() -> i32 { + static LEVEL: LazyLock = LazyLock::new(|| match std::env::var(ZSTD_LEVEL_ENV) { + Ok(s) => match s.trim().parse::() { + Ok(v) => { + let clamped = v.clamp(1, 22); + if clamped != v { + tracing::warn!( + "{ZSTD_LEVEL_ENV}={v} is outside zstd's [1, 22] range; using {clamped}" + ); + } + clamped + } + Err(_) => { + tracing::warn!( + "{ZSTD_LEVEL_ENV}={s:?} is not a valid integer; using default level \ + {DEFAULT_ZSTD_LEVEL}" + ); + DEFAULT_ZSTD_LEVEL + } + }, + Err(_) => DEFAULT_ZSTD_LEVEL, + }); + *LEVEL + } +} + +#[cfg(not(any(windows, unix)))] +mod platform { + use zarrs::storage::store::MemoryStore; + + use super::*; + + /// Open an in-memory store. This target has no filesystem; the web frontend uses it as a + /// no-op sink that's dropped at session end. + pub fn open_store(_path: &Path) -> anyhow::Result { + Ok(Arc::new(MemoryStore::new())) + } + + /// The in-memory store has no on-disk `zarr.json`, so the root group is always (re)built (the + /// memory store silently overwrites any existing entry). + pub fn root_group_missing(_path: &Path) -> bool { + true + } + + /// Stream-tier codec chain: CRC32C only. The `zstd` feature isn't built on this target (its + /// `zstd-sys` C build expects POSIX symbols the wasm libc shim doesn't expose), and the + /// memory-backed store is ephemeral, so skipping compression is fine. + pub fn stream_tier_codecs() -> Vec> { + vec![Arc::new(Crc32cCodec::new())] + } + + // The in-memory store wraps a trait object bounded only by `MaybeSend + MaybeSync` (no-ops on + // wasm), so the `Arc` isn't `Send`/`Sync` in Rust's eyes. The rest of `ext` requires chain + // complexes to be `Send + Sync`, and rippling that relaxation through the whole crate to + // accommodate the wasm frontend isn't worth it. We force `Send + Sync` here — sound because + // this target is single-threaded, so the absent cross-thread guarantees are vacuously + // satisfied. On `any(windows, unix)` the filesystem store is already `Send + Sync`, so no impl + // is needed there. + unsafe impl Send for super::ZarrSaveStore {} + unsafe impl Sync for super::ZarrSaveStore {} +} + /// Most-negative stem the on-disk layout can store. /// /// Hidden from callers; used internally to shift caller-supplied `n` values into the unsigned @@ -125,53 +232,6 @@ const SHARD_S: u64 = 8; /// Shard shape in the idx dimension. const SHARD_IDX: u64 = 8; -/// Default stream-tier zstd level when [`ZSTD_LEVEL_ENV`] is unset. -/// -/// Level 3 is zstd's own default and sits at the knee of the ratio/speed curve. The stream tier -/// (`res_qi`, `nassau_qi`) is on the hot write path — every differential quasi-inverse is -/// compressed here — so a high level makes saving several times slower for a small extra ratio -/// on this already-dense data. Large runs that want to trade write time for maximum on-disk -/// compression can raise the level via the environment variable below. -#[cfg(not(target_arch = "wasm32"))] -const DEFAULT_ZSTD_LEVEL: i32 = 3; - -/// Environment variable that overrides the stream-tier zstd level. -/// -/// For very large resolutions (where the on-disk footprint, not save time, is the binding -/// constraint) set e.g. `EXT_SAVE_ZSTD_LEVEL=19` for near-maximum compression. The value is read -/// once, parsed as an integer, and clamped to zstd's valid `[1, 22]` range; anything unparseable -/// falls back to [`DEFAULT_ZSTD_LEVEL`] with a warning. -#[cfg(not(target_arch = "wasm32"))] -const ZSTD_LEVEL_ENV: &str = "EXT_SAVE_ZSTD_LEVEL"; - -/// Resolve the stream-tier zstd level, reading [`ZSTD_LEVEL_ENV`] at most once. -#[cfg(not(target_arch = "wasm32"))] -fn zstd_level() -> i32 { - use std::sync::LazyLock; - static LEVEL: LazyLock = LazyLock::new(|| match std::env::var(ZSTD_LEVEL_ENV) { - Ok(s) => match s.trim().parse::() { - Ok(v) => { - let clamped = v.clamp(1, 22); - if clamped != v { - tracing::warn!( - "{ZSTD_LEVEL_ENV}={v} is outside zstd's [1, 22] range; using {clamped}" - ); - } - clamped - } - Err(_) => { - tracing::warn!( - "{ZSTD_LEVEL_ENV}={s:?} is not a valid integer; using default level \ - {DEFAULT_ZSTD_LEVEL}" - ); - DEFAULT_ZSTD_LEVEL - } - }, - Err(_) => DEFAULT_ZSTD_LEVEL, - }); - *LEVEL -} - /// Convert a zarrs error into `anyhow::Error`. /// /// On `wasm32-unknown-unknown` some zarrs error types contain `Arc` with only @@ -183,26 +243,6 @@ fn zarr_err(e: E) -> anyhow::Error { anyhow::anyhow!("{e}") } -/// Build the bytes-to-bytes codec chain for stream-tier arrays. -/// -/// On native targets this is `zstd(level=19) + crc32c`. On wasm32 the `zstd` feature is -/// disabled (its C build expects POSIX symbols the wasm libc shim doesn't expose) so we fall -/// back to `crc32c` alone — the memory-backed store on wasm is ephemeral, so skipping -/// compression is fine. -fn stream_tier_codecs() -> Vec> { - #[cfg(not(target_arch = "wasm32"))] - { - vec![ - Arc::new(ZstdCodec::new(zstd_level(), false)), - Arc::new(Crc32cCodec::new()), - ] - } - #[cfg(target_arch = "wasm32")] - { - vec![Arc::new(Crc32cCodec::new())] - } -} - /// Number of matrix rows per chunk in the ResQi `rows` array. /// /// Bounds the memory needed to read one chunk worth of rows. @@ -265,13 +305,6 @@ pub struct ZarrSaveStore { /// Alias for the concrete opened-array type shared across the store and its streaming readers. type ShardArray = zarrs::array::Array; -// See the doc comment on `ZarrSaveStore::store` for why this is sound on wasm. On native the -// trait object is already `Send + Sync`, so the cfg gate avoids a redundant-impl warning. -#[cfg(target_arch = "wasm32")] -unsafe impl Send for ZarrSaveStore {} -#[cfg(target_arch = "wasm32")] -unsafe impl Sync for ZarrSaveStore {} - impl std::fmt::Debug for ZarrSaveStore { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ZarrSaveStore") @@ -286,21 +319,11 @@ impl ZarrSaveStore { let path = std::path::absolute(path.as_ref()) .with_context(|| format!("Failed to resolve path: {:?}", path.as_ref()))?; - #[cfg(not(target_arch = "wasm32"))] - let store: ReadableWritableListableStorage = Arc::new(FilesystemStore::new(&path)?); - #[cfg(target_arch = "wasm32")] - let store: ReadableWritableListableStorage = Arc::new(MemoryStore::new()); - - // Root group. - // - // On WASM the store is in-memory, so `zarr.json` never "exists" on disk; we - // unconditionally build the root group (the memory store silently overwrites any - // existing entry). - #[cfg(not(target_arch = "wasm32"))] - let needs_root_group = !path.join("zarr.json").exists(); - #[cfg(target_arch = "wasm32")] - let needs_root_group = true; - if needs_root_group { + let store: ReadableWritableListableStorage = platform::open_store(&path)?; + + // Build the root group's `zarr.json` unless it's already on disk. (On the in-memory + // target it never persists, so `platform::root_group_missing` is always true there.) + if platform::root_group_missing(&path) { GroupBuilder::new() .build(store.clone(), "/")? .store_metadata()?; @@ -713,7 +736,7 @@ impl ZarrSaveStore { data_type::int64(), zarrs::array::FillValue::from(0i64), ) - .bytes_to_bytes_codecs(stream_tier_codecs()) + .bytes_to_bytes_codecs(platform::stream_tier_codecs()) .build(self.store.clone(), &format!("{}/pivots", group_path))?; pivots_array.store_metadata()?; let mut pivots_data: Vec = match qi.pivots() { @@ -737,7 +760,7 @@ impl ZarrSaveStore { data_type::uint8(), zarrs::array::FillValue::from(0u8), ) - .bytes_to_bytes_codecs(stream_tier_codecs()) + .bytes_to_bytes_codecs(platform::stream_tier_codecs()) .build(self.store.clone(), &format!("{}/rows", group_path))?; rows_array.store_metadata()?; @@ -879,7 +902,7 @@ impl ZarrSaveStore { data_type::bytes(), zarrs::array::FillValue::from(Vec::::new()), ) - .bytes_to_bytes_codecs(stream_tier_codecs()) + .bytes_to_bytes_codecs(platform::stream_tier_codecs()) .build(self.store.clone(), &format!("{}/commands", group_path))?; commands_array.store_metadata()?; From f4360401310572135b2729dc2c4a63aab96078ff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 05:58:35 +0000 Subject: [PATCH 11/13] save: bind save directory to the resolved complex, not just the algebra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit's "bind to the complex" finding. Previously a save directory only recorded the algebra (magic + prime), so reusing one directory for a *different* module over the same algebra — e.g. resolve S_2, then point the same dir at C2 — would silently load the first module's cached differentials/kernels/quasi-inverses for the second: structurally valid, semantically wrong, and CRC-clean. - Add `ChainComplex::fingerprint()`: a stable content hash of the complex's structure (each bounded module's per-degree dimensions and the algebra action on every basis element, plus each differential's action — basis-sensitive, exactly what the resolution algorithm consumes). It walks `s` until the cached zero module (since `next_homological_degree` is `i32::MAX` for a `FiniteChainComplex`) under a safety cap, and skips unbounded modules. Hashing uses a fixed FNV-1a so the value is stable across runs and toolchains, unlike `std::hash::DefaultHasher`. - Rename `ZarrSaveStore::bind_to_algebra` -> `bind_to_complex`, taking the fingerprint and storing it (as a hex string, to survive the JSON round-trip exactly). On resume the stored fingerprint must match; a mismatch — including a store predating this attribute — fails loudly instead of mixing data. - New `test_wrong_complex`: S_2 then C2 in the same dir errors with "different complex". Existing save/load + resume tests still pass (same complex hashes equal, so resume is unaffected). --- ext/src/chain_complex/mod.rs | 144 +++++++++++++++++++++++++++++- ext/src/nassau.rs | 7 +- ext/src/resolution.rs | 7 +- ext/src/save.rs | 35 ++++++-- ext/tests/save_load_resolution.rs | 15 ++++ 5 files changed, 199 insertions(+), 9 deletions(-) diff --git a/ext/src/chain_complex/mod.rs b/ext/src/chain_complex/mod.rs index 958c5c5248..e2f272c407 100644 --- a/ext/src/chain_complex/mod.rs +++ b/ext/src/chain_complex/mod.rs @@ -15,8 +15,8 @@ pub use chain_homotopy::ChainHomotopy; pub use finite_chain_complex::{FiniteAugmentedChainComplex, FiniteChainComplex}; use fp::{ matrix::Matrix, - prime::ValidPrime, - vector::{FpSlice, FpSliceMut}, + prime::{Prime, ValidPrime}, + vector::{FpSlice, FpSliceMut, FpVector}, }; use itertools::Itertools; use sseq::coordinates::{Bidegree, BidegreeGenerator}; @@ -28,6 +28,38 @@ pub enum ChainComplexGrading { Cohomological, } +/// A tiny FNV-1a 64-bit accumulator used by [`ChainComplex::fingerprint`]. +/// +/// `std`'s `DefaultHasher` is not guaranteed stable across toolchain versions, which would +/// spuriously invalidate an on-disk save directory after a compiler upgrade. FNV-1a is a fixed, +/// specified algorithm, so the fingerprint is reproducible across runs and builds. +struct Fnv(u64); + +impl Fnv { + fn new() -> Self { + Self(0xcbf2_9ce4_8422_2325) + } + + fn write_u64(&mut self, x: u64) { + for b in x.to_le_bytes() { + self.0 ^= u64::from(b); + self.0 = self.0.wrapping_mul(0x0000_0100_0000_01b3); + } + } + + fn write_i32(&mut self, x: i32) { + self.write_u64(u64::from(x as u32)); + } + + fn write_usize(&mut self, x: usize) { + self.write_u64(x as u64); + } + + fn finish(self) -> u64 { + self.0 + } +} + pub trait FreeChainComplex: ChainComplex< Module = MuFreeModule::Algebra>, @@ -220,6 +252,114 @@ pub trait ChainComplex: Send + Sync { /// The first s such that `self.module(s)` is not defined. fn next_homological_degree(&self) -> i32; + /// A stable content hash of this complex's structure. + /// + /// This is used to bind a save directory to the *specific complex* being resolved, not just + /// to its algebra. Reusing a save directory for a different complex over the same algebra + /// would otherwise silently load structurally-valid but wrong cached data. Two complexes that + /// yield the same resolution hash equal; two that differ hash differently with overwhelming + /// probability. + /// + /// The hash covers, over the complex's modules, each module's per-degree dimensions and the + /// algebra action on every basis element, plus each differential's action — so it is + /// basis-sensitive, exactly matching what the resolution algorithm consumes. + /// + /// It only makes sense for the *bounded* augmentation complexes that actually get resolved. + /// Since [`next_homological_degree`](ChainComplex::next_homological_degree) is unbounded for a + /// [`FiniteChainComplex`] (it returns `i32::MAX`), we walk `s` until [`module`](ChainComplex::module) + /// returns the complex's cached zero module (the trailing padding of a bounded complex), + /// bounded by a generous safety cap. A module with no `max_degree` (i.e. unbounded) is not a + /// complex we can meaningfully fingerprint, so its structure is skipped rather than risking a + /// runaway loop. The accumulator is a fixed FNV-1a so the value is stable across runs and + /// toolchains (unlike `std`'s `DefaultHasher`). + fn fingerprint(&self) -> u64 { + /// Safety cap on the homological length so a pathological complex can never hang the + /// hash. Real augmentation complexes have a handful of modules. + const MAX_S: i32 = 1 << 20; + + let p = self.prime(); + let mut h = Fnv::new(); + h.write_u64(u64::from(p.as_u32())); + h.write_i32(self.min_degree()); + + let zero = self.zero_module(); + let mut num_modules = 0; + for s in 0..MAX_S { + let module = self.module(s); + // Trailing zero modules mark the end of a bounded complex. + if Arc::ptr_eq(&module, &zero) { + break; + } + num_modules = s + 1; + let lo = module.min_degree(); + h.write_i32(s); + h.write_i32(lo); + + let Some(hi) = module.max_degree() else { + // Unbounded module: not a resolvable complex, don't try to hash its structure. + h.write_i32(i32::MIN); + continue; + }; + h.write_i32(hi); + module.compute_basis(hi); + let algebra = module.algebra(); + + for deg in lo..=hi { + let dim = module.dimension(deg); + h.write_i32(deg); + h.write_usize(dim); + if dim == 0 { + continue; + } + // The action of every algebra basis element on every module basis element pins + // down the module structure in the chosen basis. + for op_deg in 0..=(hi - deg) { + algebra.compute_basis(op_deg); + let op_dim = algebra.dimension(op_deg); + let out_dim = module.dimension(deg + op_deg); + for op_idx in 0..op_dim { + for mod_idx in 0..dim { + let mut out = FpVector::new(p, out_dim); + module.act_on_basis( + out.as_slice_mut(), + 1, + op_deg, + op_idx, + deg, + mod_idx, + ); + for x in out.iter() { + h.write_u64(u64::from(x)); + } + } + } + } + } + + // The differential out of module(s). Zero for the `ccdz` complexes usually resolved, + // but non-trivial complexes (e.g. cofibers) must not collide with each other. + let d = self.differential(s); + let shift = d.degree_shift(); + let target = d.target(); + h.write_i32(shift); + for deg in lo..=hi { + let out_deg = deg - shift; + target.compute_basis(out_deg); + let out_dim = target.dimension(out_deg); + for idx in 0..module.dimension(deg) { + let mut out = FpVector::new(p, out_dim); + d.apply_to_basis_element(out.as_slice_mut(), 1, deg, idx); + for x in out.iter() { + h.write_u64(u64::from(x)); + } + } + } + } + h.write_i32(num_modules); + + h.finish() + } + /// Iterate through all defined bidegrees in increasing order of stem. fn iter_stem(&self) -> StemIterator<'_, Self> { StemIterator { diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index becc65b86f..1797e12871 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -347,7 +347,12 @@ impl> Resolution { let target = Arc::new(FiniteChainComplex::ccdz(module)); if let Some(store) = save_dir.store() { let algebra = target.algebra(); - store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; + store.bind_to_complex( + algebra.magic(), + algebra.prime().as_u32(), + algebra.prefix(), + target.fingerprint(), + )?; } Ok(Self { diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index 055a1e8820..7bcbda10cb 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -138,7 +138,12 @@ where let save_dir = save_dir.try_into().map_err(Into::into)?; let algebra = complex.algebra(); if let Some(store) = save_dir.store() { - store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; + store.bind_to_complex( + algebra.magic(), + algebra.prime().as_u32(), + algebra.prefix(), + complex.fingerprint(), + )?; } let min_degree = complex.min_degree(); let zero_module = Arc::new(MuFreeModule::new(algebra, "F_{-1}".to_string(), min_degree)); diff --git a/ext/src/save.rs b/ext/src/save.rs index 120ffb779c..0c845e2a80 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -339,23 +339,32 @@ impl ZarrSaveStore { }) } - /// Bind this store to a specific algebra, catching accidental algebra / prime mismatches. + /// Bind this store to a specific complex, catching accidental algebra / prime / complex + /// mismatches. /// /// On a fresh store (the root group's `algebra_magic` attribute is unset) this writes - /// `algebra_magic`, `prime`, and `algebra_prefix` to the root group so a later load can - /// verify them. On an already-bound store the stored magic is compared against - /// `algebra_magic` and a mismatch returns an error citing both values. + /// `algebra_magic`, `prime`, `algebra_prefix`, and `complex_fingerprint` to the root group so + /// a later load can verify them. On an already-bound store the stored values are compared and + /// any mismatch returns an error. + /// + /// The `complex_fingerprint` (see [`ChainComplex::fingerprint`](crate::chain_complex::ChainComplex::fingerprint)) + /// distinguishes two different complexes over the *same* algebra: without it, resuming a save + /// directory against a different module would silently load structurally-valid but wrong + /// cached differentials/kernels/quasi-inverses. It's stored as a hex string so the full 64 + /// bits survive the JSON round-trip exactly. /// /// Callers should invoke this once per resolution setup — typically from /// `Resolution::new_with_save` — before any data is read or written. Subgroups /// (`products/…`, `homotopies/…`) share the same underlying store and therefore the same /// root attributes, so they inherit the check without a second call. - pub fn bind_to_algebra( + pub fn bind_to_complex( &self, algebra_magic: u32, prime: u32, algebra_prefix: &str, + complex_fingerprint: u64, ) -> anyhow::Result<()> { + let fingerprint_hex = format!("{complex_fingerprint:016x}"); let root = zarrs::group::Group::open(self.store.clone(), "/").map_err(zarr_err)?; let attrs = root.attributes(); if let Some(stored) = attrs.get("algebra_magic").and_then(|v| v.as_u64()) { @@ -377,6 +386,21 @@ impl ZarrSaveStore { self.path, ); } + // Same algebra: the store must also have been created for the same complex. A stored + // store predating this check (no `complex_fingerprint`) is treated as a mismatch. + let stored_fingerprint = attrs + .get("complex_fingerprint") + .and_then(|v| v.as_str()) + .unwrap_or(""); + if stored_fingerprint != fingerprint_hex { + anyhow::bail!( + "Save store at {:?} was created for a different complex over the same algebra \ + ({algebra_prefix} at p={prime}): stored complex fingerprint \ + {stored_fingerprint}, expected {fingerprint_hex}. Refusing to mix cached \ + data between distinct complexes; use a separate save directory.", + self.path, + ); + } return Ok(()); } @@ -386,6 +410,7 @@ impl ZarrSaveStore { new_attrs.insert("algebra_magic".into(), (u64::from(algebra_magic)).into()); new_attrs.insert("prime".into(), (u64::from(prime)).into()); new_attrs.insert("algebra_prefix".into(), algebra_prefix.into()); + new_attrs.insert("complex_fingerprint".into(), fingerprint_hex.into()); let group = GroupBuilder::new() .attributes(new_attrs) .build(self.store.clone(), "/") diff --git a/ext/tests/save_load_resolution.rs b/ext/tests/save_load_resolution.rs index e32819f98c..4b9223d8ab 100644 --- a/ext/tests/save_load_resolution.rs +++ b/ext/tests/save_load_resolution.rs @@ -49,6 +49,21 @@ fn test_wrong_algebra() { construct_standard::("S_2@milnor", Some(tempdir.path().into())).unwrap(); } +/// Resolving one module and then reusing the same save dir for a *different* module over the same +/// algebra must fail loudly via the complex-fingerprint check, rather than silently loading the +/// first module's cached differentials for the second. +#[test] +#[should_panic(expected = "different complex")] +fn test_wrong_complex() { + let tempdir = tempfile::TempDir::new().unwrap(); + let resolution1 = + construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); + resolution1.compute_through_bidegree(Bidegree::s_t(2, 2)); + + // Same Steenrod algebra at p = 2, but a different module (the mod-2 Moore complex). + construct_standard::("C2", Some(tempdir.path().into())).unwrap(); +} + #[test] fn test_save_load_stem() { let tempdir = tempfile::TempDir::new().unwrap(); From 177ec5dd91a3590e0bb73b0ea9d8df15b2e85fe2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 06:39:54 +0000 Subject: [PATCH 12/13] save: make save directories self-describing via the recorded module spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supersedes the previous complex-fingerprint approach (commit f436040), which bound a save directory to its complex by storing an opaque 64-bit content hash. Storing the module's JSON spec instead does the same identity job while being readable, inspectable, and reusable for reconstruction — one artifact serving three purposes. - Revert `ChainComplex::fingerprint()`/`Fnv` and restore `bind_to_algebra` (algebra magic/prime/prefix). The complex-identity gate now lives in the new `ZarrSaveStore::bind_module_spec`, called from `construct`/`construct_nassau` where the spec is actually known: on a fresh store it records `module_spec`, on an existing one it must match exactly, else it bails. `test_wrong_complex` (S_2 then C2 in one dir) still fails loudly, now via the spec comparison. - `construct_from_save(dir)`: rebuild and resume a resolution from a save directory alone — reads the recorded `module_spec` + `algebra_prefix`, rebuilds the `Config`, and resolves back into the same dir. No need to re-supply what the directory already knows it holds. - `set_complex_name`: record the short human-readable label (the module spec the user typed, e.g. "S_2") as a `complex_name` attribute, a concise companion to the full `module_spec`, so `zarr.json` says what it resolves. - Fix a broken rustdoc intra-doc link (`FilesystemStore`) that failed the docs build. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AJt3nb5Fp6okurqLzcQbCa --- ext/src/chain_complex/mod.rs | 144 +----------------------------- ext/src/nassau.rs | 17 ++-- ext/src/resolution.rs | 17 ++-- ext/src/save.rs | 112 ++++++++++++++++------- ext/src/utils.rs | 53 ++++++++++- ext/tests/save_load_resolution.rs | 65 +++++++++++++- 6 files changed, 216 insertions(+), 192 deletions(-) diff --git a/ext/src/chain_complex/mod.rs b/ext/src/chain_complex/mod.rs index e2f272c407..958c5c5248 100644 --- a/ext/src/chain_complex/mod.rs +++ b/ext/src/chain_complex/mod.rs @@ -15,8 +15,8 @@ pub use chain_homotopy::ChainHomotopy; pub use finite_chain_complex::{FiniteAugmentedChainComplex, FiniteChainComplex}; use fp::{ matrix::Matrix, - prime::{Prime, ValidPrime}, - vector::{FpSlice, FpSliceMut, FpVector}, + prime::ValidPrime, + vector::{FpSlice, FpSliceMut}, }; use itertools::Itertools; use sseq::coordinates::{Bidegree, BidegreeGenerator}; @@ -28,38 +28,6 @@ pub enum ChainComplexGrading { Cohomological, } -/// A tiny FNV-1a 64-bit accumulator used by [`ChainComplex::fingerprint`]. -/// -/// `std`'s `DefaultHasher` is not guaranteed stable across toolchain versions, which would -/// spuriously invalidate an on-disk save directory after a compiler upgrade. FNV-1a is a fixed, -/// specified algorithm, so the fingerprint is reproducible across runs and builds. -struct Fnv(u64); - -impl Fnv { - fn new() -> Self { - Self(0xcbf2_9ce4_8422_2325) - } - - fn write_u64(&mut self, x: u64) { - for b in x.to_le_bytes() { - self.0 ^= u64::from(b); - self.0 = self.0.wrapping_mul(0x0000_0100_0000_01b3); - } - } - - fn write_i32(&mut self, x: i32) { - self.write_u64(u64::from(x as u32)); - } - - fn write_usize(&mut self, x: usize) { - self.write_u64(x as u64); - } - - fn finish(self) -> u64 { - self.0 - } -} - pub trait FreeChainComplex: ChainComplex< Module = MuFreeModule::Algebra>, @@ -252,114 +220,6 @@ pub trait ChainComplex: Send + Sync { /// The first s such that `self.module(s)` is not defined. fn next_homological_degree(&self) -> i32; - /// A stable content hash of this complex's structure. - /// - /// This is used to bind a save directory to the *specific complex* being resolved, not just - /// to its algebra. Reusing a save directory for a different complex over the same algebra - /// would otherwise silently load structurally-valid but wrong cached data. Two complexes that - /// yield the same resolution hash equal; two that differ hash differently with overwhelming - /// probability. - /// - /// The hash covers, over the complex's modules, each module's per-degree dimensions and the - /// algebra action on every basis element, plus each differential's action — so it is - /// basis-sensitive, exactly matching what the resolution algorithm consumes. - /// - /// It only makes sense for the *bounded* augmentation complexes that actually get resolved. - /// Since [`next_homological_degree`](ChainComplex::next_homological_degree) is unbounded for a - /// [`FiniteChainComplex`] (it returns `i32::MAX`), we walk `s` until [`module`](ChainComplex::module) - /// returns the complex's cached zero module (the trailing padding of a bounded complex), - /// bounded by a generous safety cap. A module with no `max_degree` (i.e. unbounded) is not a - /// complex we can meaningfully fingerprint, so its structure is skipped rather than risking a - /// runaway loop. The accumulator is a fixed FNV-1a so the value is stable across runs and - /// toolchains (unlike `std`'s `DefaultHasher`). - fn fingerprint(&self) -> u64 { - /// Safety cap on the homological length so a pathological complex can never hang the - /// hash. Real augmentation complexes have a handful of modules. - const MAX_S: i32 = 1 << 20; - - let p = self.prime(); - let mut h = Fnv::new(); - h.write_u64(u64::from(p.as_u32())); - h.write_i32(self.min_degree()); - - let zero = self.zero_module(); - let mut num_modules = 0; - for s in 0..MAX_S { - let module = self.module(s); - // Trailing zero modules mark the end of a bounded complex. - if Arc::ptr_eq(&module, &zero) { - break; - } - num_modules = s + 1; - let lo = module.min_degree(); - h.write_i32(s); - h.write_i32(lo); - - let Some(hi) = module.max_degree() else { - // Unbounded module: not a resolvable complex, don't try to hash its structure. - h.write_i32(i32::MIN); - continue; - }; - h.write_i32(hi); - module.compute_basis(hi); - let algebra = module.algebra(); - - for deg in lo..=hi { - let dim = module.dimension(deg); - h.write_i32(deg); - h.write_usize(dim); - if dim == 0 { - continue; - } - // The action of every algebra basis element on every module basis element pins - // down the module structure in the chosen basis. - for op_deg in 0..=(hi - deg) { - algebra.compute_basis(op_deg); - let op_dim = algebra.dimension(op_deg); - let out_dim = module.dimension(deg + op_deg); - for op_idx in 0..op_dim { - for mod_idx in 0..dim { - let mut out = FpVector::new(p, out_dim); - module.act_on_basis( - out.as_slice_mut(), - 1, - op_deg, - op_idx, - deg, - mod_idx, - ); - for x in out.iter() { - h.write_u64(u64::from(x)); - } - } - } - } - } - - // The differential out of module(s). Zero for the `ccdz` complexes usually resolved, - // but non-trivial complexes (e.g. cofibers) must not collide with each other. - let d = self.differential(s); - let shift = d.degree_shift(); - let target = d.target(); - h.write_i32(shift); - for deg in lo..=hi { - let out_deg = deg - shift; - target.compute_basis(out_deg); - let out_dim = target.dimension(out_deg); - for idx in 0..module.dimension(deg) { - let mut out = FpVector::new(p, out_dim); - d.apply_to_basis_element(out.as_slice_mut(), 1, deg, idx); - for x in out.iter() { - h.write_u64(u64::from(x)); - } - } - } - } - h.write_i32(num_modules); - - h.finish() - } - /// Iterate through all defined bidegrees in increasing order of stem. fn iter_stem(&self) -> StemIterator<'_, Self> { StemIterator { diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index 1797e12871..dc072d1ca5 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -329,6 +329,16 @@ impl> Resolution { } pub fn set_name(&mut self, name: String) { + // Record the label in the save store (if any) so the on-disk `zarr.json` is + // self-describing. Best-effort and purely informational — the module spec written by + // `bind_module_spec` is what actually guards loading. + if !name.is_empty() + && let Some(store) = self.save_dir.store() + { + store + .set_complex_name(&name) + .expect("Failed to record complex name in save store"); + } self.name = name; } @@ -347,12 +357,7 @@ impl> Resolution { let target = Arc::new(FiniteChainComplex::ccdz(module)); if let Some(store) = save_dir.store() { let algebra = target.algebra(); - store.bind_to_complex( - algebra.magic(), - algebra.prime().as_u32(), - algebra.prefix(), - target.fingerprint(), - )?; + store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; } Ok(Self { diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index 7bcbda10cb..b4673bb6b0 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -138,12 +138,7 @@ where let save_dir = save_dir.try_into().map_err(Into::into)?; let algebra = complex.algebra(); if let Some(store) = save_dir.store() { - store.bind_to_complex( - algebra.magic(), - algebra.prime().as_u32(), - algebra.prefix(), - complex.fingerprint(), - )?; + store.bind_to_algebra(algebra.magic(), algebra.prime().as_u32(), algebra.prefix())?; } let min_degree = complex.min_degree(); let zero_module = Arc::new(MuFreeModule::new(algebra, "F_{-1}".to_string(), min_degree)); @@ -165,6 +160,16 @@ where } pub fn set_name(&mut self, name: String) { + // Record the label in the save store (if any) so the on-disk `zarr.json` is + // self-describing. Best-effort and purely informational — the module spec written by + // `bind_module_spec` is what actually guards loading. + if !name.is_empty() + && let Some(store) = self.save_dir.store() + { + store + .set_complex_name(&name) + .expect("Failed to record complex name in save store"); + } self.name = name; } diff --git a/ext/src/save.rs b/ext/src/save.rs index 0c845e2a80..63bc1a03c2 100644 --- a/ext/src/save.rs +++ b/ext/src/save.rs @@ -54,7 +54,7 @@ //! `EXT_SAVE_ZSTD_LEVEL` environment variable for very large runs where the on-disk footprint //! matters more than save time. //! -//! Subgroups share the same underlying [`FilesystemStore`] via `Arc` clone; only the `group` +//! Subgroups share the same underlying `FilesystemStore` via `Arc` clone; only the `group` //! prefix differs. Shard arrays are created lazily on first write so that subgroups don't //! populate kinds they never use. //! @@ -339,32 +339,28 @@ impl ZarrSaveStore { }) } - /// Bind this store to a specific complex, catching accidental algebra / prime / complex - /// mismatches. + /// Bind this store to a specific algebra, catching accidental algebra / prime mismatches. /// /// On a fresh store (the root group's `algebra_magic` attribute is unset) this writes - /// `algebra_magic`, `prime`, `algebra_prefix`, and `complex_fingerprint` to the root group so - /// a later load can verify them. On an already-bound store the stored values are compared and - /// any mismatch returns an error. + /// `algebra_magic`, `prime`, and `algebra_prefix` to the root group so a later load can + /// verify them. On an already-bound store the stored magic is compared against + /// `algebra_magic` and a mismatch returns an error citing both values. /// - /// The `complex_fingerprint` (see [`ChainComplex::fingerprint`](crate::chain_complex::ChainComplex::fingerprint)) - /// distinguishes two different complexes over the *same* algebra: without it, resuming a save - /// directory against a different module would silently load structurally-valid but wrong - /// cached differentials/kernels/quasi-inverses. It's stored as a hex string so the full 64 - /// bits survive the JSON round-trip exactly. + /// This guards only the *algebra*; the *module* is pinned separately by + /// [`Self::bind_module_spec`] (e.g. `S_2` vs `C2` share this algebra but are distinct + /// complexes). `algebra_prefix` is also what [`construct_from_save`](crate::utils::construct_from_save) + /// reads back to pick the algebra when reconstructing. /// /// Callers should invoke this once per resolution setup — typically from /// `Resolution::new_with_save` — before any data is read or written. Subgroups /// (`products/…`, `homotopies/…`) share the same underlying store and therefore the same /// root attributes, so they inherit the check without a second call. - pub fn bind_to_complex( + pub fn bind_to_algebra( &self, algebra_magic: u32, prime: u32, algebra_prefix: &str, - complex_fingerprint: u64, ) -> anyhow::Result<()> { - let fingerprint_hex = format!("{complex_fingerprint:016x}"); let root = zarrs::group::Group::open(self.store.clone(), "/").map_err(zarr_err)?; let attrs = root.attributes(); if let Some(stored) = attrs.get("algebra_magic").and_then(|v| v.as_u64()) { @@ -386,21 +382,6 @@ impl ZarrSaveStore { self.path, ); } - // Same algebra: the store must also have been created for the same complex. A stored - // store predating this check (no `complex_fingerprint`) is treated as a mismatch. - let stored_fingerprint = attrs - .get("complex_fingerprint") - .and_then(|v| v.as_str()) - .unwrap_or(""); - if stored_fingerprint != fingerprint_hex { - anyhow::bail!( - "Save store at {:?} was created for a different complex over the same algebra \ - ({algebra_prefix} at p={prime}): stored complex fingerprint \ - {stored_fingerprint}, expected {fingerprint_hex}. Refusing to mix cached \ - data between distinct complexes; use a separate save directory.", - self.path, - ); - } return Ok(()); } @@ -410,7 +391,6 @@ impl ZarrSaveStore { new_attrs.insert("algebra_magic".into(), (u64::from(algebra_magic)).into()); new_attrs.insert("prime".into(), (u64::from(prime)).into()); new_attrs.insert("algebra_prefix".into(), algebra_prefix.into()); - new_attrs.insert("complex_fingerprint".into(), fingerprint_hex.into()); let group = GroupBuilder::new() .attributes(new_attrs) .build(self.store.clone(), "/") @@ -419,6 +399,78 @@ impl ZarrSaveStore { Ok(()) } + /// Record a human-readable label for the complex this store resolves, as a `complex_name` + /// attribute on the root group. + /// + /// Purely informational: unlike [`Self::bind_module_spec`], this is never used to gate loading — + /// names are free-form and often empty. It's a concise companion to the full `module_spec`, so + /// someone inspecting `zarr.json` sees what the save is a resolution of at a glance. Overwrites + /// any previous label. + pub fn set_complex_name(&self, name: &str) -> anyhow::Result<()> { + let root = zarrs::group::Group::open(self.store.clone(), "/").map_err(zarr_err)?; + let mut new_attrs = root.attributes().clone(); + new_attrs.insert("complex_name".into(), name.into()); + let group = GroupBuilder::new() + .attributes(new_attrs) + .build(self.store.clone(), "/") + .map_err(zarr_err)?; + group.store_metadata().map_err(zarr_err)?; + Ok(()) + } + + /// Bind this store to a specific module, and record the spec so the directory is + /// self-describing. + /// + /// This is the complex-identity gate that complements [`Self::bind_to_algebra`]: reusing a save + /// directory for a *different* module over the same algebra — resolve `S_2`, then point the + /// same dir at `C2` — is rejected here rather than silently loading the first module's cached + /// data for the second. On a fresh store the spec is written; on an already-bound store it must + /// match the stored one exactly, otherwise an error is returned. + /// + /// The recorded `module_spec` is the same JSON that + /// [`construct_from_save`](crate::utils::construct_from_save) reads to rebuild the complex from + /// the directory alone — one readable, inspectable artifact serving identity, reconstruction, + /// and documentation, in place of an opaque content hash. + pub fn bind_module_spec(&self, spec: &serde_json::Value) -> anyhow::Result<()> { + let root = zarrs::group::Group::open(self.store.clone(), "/").map_err(zarr_err)?; + let attrs = root.attributes(); + if let Some(stored) = attrs.get("module_spec") { + if stored != spec { + anyhow::bail!( + "Save store at {:?} was created for a different complex: the stored module \ + spec does not match the one being resolved. Refusing to mix cached data \ + between distinct complexes; use a separate save directory.\n stored: \ + {stored}\n expected: {spec}", + self.path, + ); + } + return Ok(()); + } + + let mut new_attrs = attrs.clone(); + new_attrs.insert("module_spec".into(), spec.clone()); + let group = GroupBuilder::new() + .attributes(new_attrs) + .build(self.store.clone(), "/") + .map_err(zarr_err)?; + group.store_metadata().map_err(zarr_err)?; + Ok(()) + } + + /// Read the root-group attributes of an existing store at `path` without binding to a complex. + /// + /// Lets a caller inspect what a save is (its `module_spec`, `algebra_prefix`, `complex_name`, + /// …) before reconstructing it. Returns the raw attribute map. + pub fn read_root_attributes( + path: impl AsRef, + ) -> anyhow::Result> { + let path = std::path::absolute(path.as_ref()) + .with_context(|| format!("Failed to resolve path: {:?}", path.as_ref()))?; + let store = platform::open_store(&path)?; + let root = zarrs::group::Group::open(store, "/").map_err(zarr_err)?; + Ok(root.attributes().clone()) + } + /// Open a subgroup at `{self.group}/{name}`. /// /// Shares the same underlying store as `self`. The subgroup's `zarr.json` is created if diff --git a/ext/src/utils.rs b/ext/src/utils.rs index 539638c158..8f2cadd267 100644 --- a/ext/src/utils.rs +++ b/ext/src/utils.rs @@ -1,5 +1,8 @@ //! A module containing various utility functions related to user interaction in some way. -use std::{path::PathBuf, sync::Arc}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; use algebra::{ AlgebraType, MilnorAlgebra, SteenrodAlgebra, @@ -13,7 +16,7 @@ use crate::{ CCC, chain_complex::{AugmentedChainComplex, BoundedChainComplex, ChainComplex, FiniteChainComplex}, resolution::{Resolution, UnstableResolution}, - save::SaveDirectory, + save::{SaveDirectory, ZarrSaveStore}, }; // We build docs with --all-features so the docs are at the feature = "nassau" version @@ -194,7 +197,9 @@ where if !json["cofiber"].is_null() { return Err(anyhow!("Nassau's algorithm does not support cofiber")); } - crate::nassau::Resolution::new_with_save(module, save_dir) + let resolution = crate::nassau::Resolution::new_with_save(module, save_dir)?; + bind_module_spec(resolution.save_dir(), &json)?; + Ok(resolution) } /// See [`construct`] @@ -281,7 +286,47 @@ where chain_complex = Arc::new(yoneda.map(|m| steenrod_module::erase(m.clone()))); } - crate::resolution::MuResolution::new_with_save(chain_complex, save_dir) + let resolution = crate::resolution::MuResolution::new_with_save(chain_complex, save_dir)?; + bind_module_spec(resolution.save_dir(), &json)?; + Ok(resolution) +} + +/// Pin the save store (if any) to this module spec and record it, so the directory both rejects +/// reuse for a different complex and is self-describing. Propagates the identity-mismatch error. +fn bind_module_spec(save_dir: &SaveDirectory, spec: &Value) -> anyhow::Result<()> { + if let Some(store) = save_dir.store() { + store.bind_module_spec(spec)?; + } + Ok(()) +} + +/// Reconstruct a resolution from a save directory alone, without re-supplying the module spec. +/// +/// Reads the `module_spec` and `algebra_prefix` that [`construct`] recorded on the store at +/// creation time, rebuilds the [`Config`], and resolves back into the same directory. The rebuilt +/// spec is re-checked against the stored one by +/// [`bind_module_spec`](crate::save::ZarrSaveStore::bind_module_spec), so a directory whose +/// contents don't match its recorded spec is rejected rather than silently mixing data. +/// +/// Errors if the directory carries no recorded spec — e.g. it was created through the raw +/// [`MuResolution::new_with_save`](crate::resolution::MuResolution::new_with_save) API with a +/// programmatically-built complex rather than through [`construct`]. +pub fn construct_from_save(save_dir: impl AsRef) -> anyhow::Result { + let save_dir = save_dir.as_ref().to_owned(); + let attrs = ZarrSaveStore::read_root_attributes(&save_dir) + .with_context(|| format!("Failed to open save store at {save_dir:?}"))?; + let module = attrs.get("module_spec").cloned().with_context(|| { + format!( + "Save store at {save_dir:?} has no recorded module spec; it was not created through \ + `construct`, so the complex cannot be reconstructed from the directory alone" + ) + })?; + let algebra = attrs + .get("algebra_prefix") + .and_then(|v| v.as_str()) + .with_context(|| format!("Save store at {save_dir:?} has no recorded algebra"))?; + let config: Config = (module, algebra).try_into()?; + construct(config, Some(save_dir)) } /// Load a module specification from a JSON file. diff --git a/ext/tests/save_load_resolution.rs b/ext/tests/save_load_resolution.rs index 4b9223d8ab..ebfab0705f 100644 --- a/ext/tests/save_load_resolution.rs +++ b/ext/tests/save_load_resolution.rs @@ -50,7 +50,7 @@ fn test_wrong_algebra() { } /// Resolving one module and then reusing the same save dir for a *different* module over the same -/// algebra must fail loudly via the complex-fingerprint check, rather than silently loading the +/// algebra must fail loudly via the recorded module-spec check, rather than silently loading the /// first module's cached differentials for the second. #[test] #[should_panic(expected = "different complex")] @@ -168,9 +168,12 @@ fn test_load_secondary() { fn test_zarr_store_exists() { let tempdir = tempfile::TempDir::new().unwrap(); - construct_standard::("S_2", Some(tempdir.path().into())) - .unwrap() - .compute_through_bidegree(Bidegree::s_t(3, 3)); + let mut resolution = + construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); + // Naming the resolution records a human-readable label on the store, mirroring how the CLI + // labels its resolutions. + resolution.set_name("S_2".into()); + resolution.compute_through_bidegree(Bidegree::s_t(3, 3)); // Verify data was stored in zarr format let store_path = tempdir.path(); @@ -182,4 +185,58 @@ fn test_zarr_store_exists() { store_path.join("differential/zarr.json").exists(), "shard-tier differential array missing" ); + + // The store is self-describing: the module spec (the identity gate + reconstruction source) + // and the human-readable label are both recorded on the root group, alongside the algebra + // binding. + let root_meta = std::fs::read_to_string(store_path.join("zarr.json")).unwrap(); + assert!( + root_meta.contains("module_spec"), + "root group should record the module spec; got: {root_meta}" + ); + assert!( + root_meta.contains("complex_name") && root_meta.contains("S_2"), + "root group should record the complex name; got: {root_meta}" + ); + assert!( + root_meta.contains("algebra_magic"), + "recording the spec must not drop the algebra binding; got: {root_meta}" + ); +} + +/// A save created through `construct` is self-describing: `construct_from_save` can rebuild the +/// complex from the directory alone, without the caller re-supplying the `"S_2"` spec, and the +/// reconstructed resolution resumes the cached data. +#[test] +fn test_construct_from_save() { + use ext::utils::{construct_from_save, construct_standard}; + + let tempdir = tempfile::TempDir::new().unwrap(); + + let resolution1 = + construct_standard::("S_2", Some(tempdir.path().into())).unwrap(); + resolution1.compute_through_stem(Bidegree::n_s(10, 6)); + + // Reopen the directory without telling the code what module it holds. + let resolution2 = construct_from_save(tempdir.path()).unwrap(); + resolution2.compute_through_stem(Bidegree::n_s(10, 6)); + + assert_eq!( + resolution1.graded_dimension_string(), + resolution2.graded_dimension_string() + ); + + // Extending past the saved range still works, confirming a genuine resumable resolution + // rather than a read-only view. + resolution2.compute_through_stem(Bidegree::n_s(14, 8)); +} + +/// Pointing `construct_from_save` at a directory that was never populated (no recorded spec) is a +/// clean error, not a panic. +#[test] +fn test_construct_from_save_missing_spec() { + use ext::utils::construct_from_save; + + let tempdir = tempfile::TempDir::new().unwrap(); + assert!(construct_from_save(tempdir.path()).is_err()); } From f189fc69b6dca543a618157a75dbafd6615a60b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 06:53:14 +0000 Subject: [PATCH 13/13] save: warn instead of panic on best-effort complex-name write The `set_name` doc comment calls the `set_complex_name` store write "best-effort and purely informational", but it used `.expect()`, so a transient store failure would abort the whole resolution over a label that doesn't guard anything (the real load guard is `bind_module_spec`, which still propagates its errors). Log a warning and continue instead. Applies to both the standard and Nassau `set_name`. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AJt3nb5Fp6okurqLzcQbCa --- ext/src/nassau.rs | 5 ++--- ext/src/resolution.rs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ext/src/nassau.rs b/ext/src/nassau.rs index dc072d1ca5..1287666311 100644 --- a/ext/src/nassau.rs +++ b/ext/src/nassau.rs @@ -334,10 +334,9 @@ impl> Resolution { // `bind_module_spec` is what actually guards loading. if !name.is_empty() && let Some(store) = self.save_dir.store() + && let Err(e) = store.set_complex_name(&name) { - store - .set_complex_name(&name) - .expect("Failed to record complex name in save store"); + tracing::warn!("Failed to record complex name in save store: {e}"); } self.name = name; } diff --git a/ext/src/resolution.rs b/ext/src/resolution.rs index b4673bb6b0..6f6c711194 100644 --- a/ext/src/resolution.rs +++ b/ext/src/resolution.rs @@ -165,10 +165,9 @@ where // `bind_module_spec` is what actually guards loading. if !name.is_empty() && let Some(store) = self.save_dir.store() + && let Err(e) = store.set_complex_name(&name) { - store - .set_complex_name(&name) - .expect("Failed to record complex name in save store"); + tracing::warn!("Failed to record complex name in save store: {e}"); } self.name = name; }