Repository navigation
One indexing modality: port of C++ SSHash v6.0.0 (0.7.0, index format 5.0) - #5
Merged
Merged
Conversation
Two K>=33 bugs, both silent until now because nothing streamed wide K:
- try_extend extracted the newest base via to_u64(bits) >> (2*(K-1)):
to_u64 truncates u128 storage and the shift overflows u64 for K >= 33
(debug panic, masked garbage in release -> possible false-positive
extension). Analogue of C++ sshash 87cab5f. Extract via get_base,
which shifts in storage width.
- append_base built its K-base mask as (1u64 << 2K) - 1, overflowing
for K >= 33; build it in u128 and truncate through from_u128.
New tests/u128_streaming.rs streams both strands at K in {33,47,63}
and asserts every streamed LookupResult equals the point lookup, with
num_extensions > 0 so the extension path is provably exercised.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
Port of C++ SSHash v6.0.0 (jermp/sshash 6c53ae9 + b1d0706 + 2cafe28 +
c22c897). The minimizer of a k-mer x is now the locus i minimizing
h(kappa(i)), kappa(i) = min(mmer_i, rc(mmer_i)): mirror-equivariant (x
and rc(x) share a bucket, one probe, orientation reported) at plain
forward density 2/(k-m+2). The old canonical mode paid a 4/3 super-kmer
density factor for the same answers; the regular mode answered less.
Both are gone: one build path, one lookup path, one streaming query.
- minimizer.rs: rewritten around canonical_mmer_at (one whole-kmer RC +
shifts via rc(x_i) = rc(x)_{K-m-i}), shared compute_minimizer batch
reference, centre-closest tie-break (Cologni & Pibiri Prop. 24 —
forward AND mirror-equivariant; outlined resolve_tie), incremental
MinimizerIterator with num_mins tie tracking and a debug assert
against the batch reference on every call. MinimizerIteratorRc
deleted.
- builder: both tuple-extraction paths collapse to the single iterator
(the old canonical branch ran a fresh full-rescan RC iterator per
k-mer); forwardness is enforced where the index is sized — duplicate
(minimizer, pos_in_seq) aborts the build ('the minimizer scheme is
not forward') in classification, external merge, and FileBucketIter;
all dedupe loops become straight writes.
- dictionary.rs: the regular/canonical dual lookup family (~300 lines)
becomes one lookup_with_minimizer core — single probe, exactly two
candidate starts j - p and j - (K-m-p); new m-mer presence check
(decode_mmer_at) makes minimizer_found reliable for non-heavy
buckets (heavy stays true, port of the C++ HEAVYLOAD rule); skew
MPHF keys are canonical unconditionally. Fixes a latent seed bug:
queries now hash with the build seed instead of hard-coded 1.
- streaming_query.rs: single minimizer iterator (RC k-mer is already
maintained); same-minimizer memos ported — minimizer-absent negative,
singleton same-occurrence skip (falls through for self-RC minimizers,
even m), and a <=8-entry decoded-positions bucket cache verified via
lookup_at_positions; seed outlined; debug assert streaming == point
on every call; new counters exposed in StreamingQueryStats.
- serialization: FORMAT_VERSION (4,0) -> (5,0), indices must be
rebuilt; header gains seed + hasher_magic (recomputed and compared on
load, so a rapidhash behavior change fails loudly instead of
silently corrupting minimizer selection — the guard flagged in
Cargo.toml's rapidhash pin comment).
- API: BuildConfiguration.canonical and Dictionary::canonical()
deprecated no-ops; StreamingQuery::new keeps its 3-arg signature
(flag ignored) + new with_seed; Dictionary::new takes seed instead
of canonical; CLI --canonical warns.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
…minism) - tests/minimizer_properties.rs: independent from-the-spec brute-force reference checked against compute_minimizer and MinimizerIterator — mirror-equivariance, incremental==batch==reference on every window, forwardness (sampled position never decreases), and a ties-actually- fire guard at small m so the tie-break cannot silently go untested. Default grid runs in ~1s; heavy grid behind --ignored (verified in release). - tests/dictionary_roundtrip.rs: full save/load LookupResult equality (both strands + absent probes), non-default-seed roundtrip (regression for the seed bug), and (4,0)-era header rejection with a 'rebuild' message. - tests/build_paths_and_determinism.rs: in-memory vs forced external- sort builds answer identically AND serialize byte-identically; two builds of the same input are byte-identical. New hidden BuildConfiguration.force_external_sort test hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
…ript - dump now prints all LookupResult fields and supports --point (point lookups instead of the streaming engine), making per-kmer dumps byte-diffable across versions and implementations. - scripts/oracle_diff.sh: the old-vs-new equivalence gate. Run against a 0.6.4 oracle binary (same dump patch on a throwaway worktree branch): builds old-canonical and new indices on shared unitig inputs (salmonella/ecoli k31 at m=13/16/20; se.ust k47/k63 at m=21) and diffs per-kmer dumps for self-queries and error-injected reads. Result on all cases (~30M lines): old point == new point byte-identical; new stream == new point on the six semantic fields; old stream == new stream for K<=31. At K>31 the OLD streaming was producing wrong results (the release-mode wide-K truncation bug this port fixes) — verified wrong against its own point lookups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
Compile against a C++ sshash v6.0.0 checkout (include dirs per its CMakeLists, link libsshash_static + z) to dump per-kmer lookup results in the same TSV as `sshash dump --point`. Cross-diff result: byte- identical to the Rust port on salmonella/ecoli k31 (m=16) and se.ust.k47/k63 (m=21, 128-bit build), self-queries and error reads — all 9 columns, >10M kmers. Their test_minimizer and build --check suites also pass on the shared inputs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
…e scan loop - Point lookups (no bucket cache) verify straight off the offsets array instead of zeroing a 512-byte stack buffer per call; the decoded locate set is written directly into the streaming cache when one is provided. - compute_minimizer's hot loop matches C++'s shape: the forward m-mer comes from a running window (constant shift), only the RC pays a variable shift. Byte-identical indices and dumps. Measured on salmonella k31 m16 vs the 0.6.4 canonical baseline (best-of-3): build 0.43s -> 0.35s (-19%, peak RSS -14%); streaming 15.1 -> 10.0 ns/kmer on 1%-error reads and 8.9 -> 5.9 on error-free positive streams (-33%); total index bytes -8..-13% across the case grid. Point lookups are ~100 ns/kmer vs 79 for the old dual-scan canonical path (the memo-soundness presence check accounts for ~5 ns; the rest is the scheme's tie tracking) — still 25% faster than C++ v6 on the same machine/data, and point lookups are not on the piscem/ salmon hot path (streaming is). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
Adds the repo's first test workflow: debug tests (all invariant asserts active), release tests, the heavy minimizer property grid, and an informational clippy pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
Addresses the newer-toolchain lints properly instead of softening the gate: darray enumerate + needless return, manual div_ceil in offsets, redundant cast, as_bytes-after-slice in the CLI dump, auto-deref in sshash-py, hex-literal grouping in the prefilter test, and every in-workspace use of the deprecated canonical field/method (tests build with the default config now; stale canonical-mode doc comments in minimizer_tuples updated to describe the unified scheme). The only remaining note is cargo's future-incompat report for proc-macro-error2 (transitive via sux -> impl-tools; upstream). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
…at 0.6.4 The point-lookup regression (79 -> 100 ns/kmer) was not the scheme's extra work — the new code executes 10% FEWER instructions than the old dual-scan canonical path. It was branch misprediction: the old loop's single 'if hash < min' with three assignments if-converts to cmovs, but the tie-tracking turned it into an 'if < / else if ==' chain, which LLVM leaves as real branches on a data-dependent comparison (measured: IPC 5.10 -> 3.66, branch misses 17.6M -> 114.2M over 24M k-mers, ~4 extra misses x ~20 cycles = the whole gap). Writing the update branchlessly (selects -> cmovs, tie/rightmost as flag arithmetic) in both compute_minimizer and the iterator's rescan: point lookups 77.7 ns/kmer (old dual-scan: 79.1; C++ v6: 134 on this machine), streaming 9.9 ns/kmer and build time unchanged, branch misses back to 13.4M (below the old baseline). Index bytes and per-kmer dumps are byte-identical; full test suite + property grid pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
Sweep of options that lost meaning under the unified canonical scheme; per policy they stay accepted for one release and warn when used: - sshash-py: forward_only=True on Dictionary.lookup/query/contains and the BuildConfig.canonical setter now emit Python DeprecationWarnings (results unaffected); docstrings updated. Also fixes the lookup() docstring, which claimed the global k-mer ID while the method has always returned the absolute base offset (kmer_offset). - sshash-cli: --forward-only help text now says deprecated/no-effect (the runtime warnings for --canonical and --forward-only were already in place). - crates/sshash-py/tests/test_basic.py: self-contained pytest suite (build from python, fwd/rc/absent point queries, streaming==point, save/load roundtrip, warning assertions) — 7 tests, run via maturin develop && pytest. Runtime-validated against the cross-checked salmonella index: 500 fwd + 500 rc python point queries match the oracle dump field-for-field, streaming matches with extensions active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
The previous commit wrongly deprecated --forward-only/forward_only= as
no-ops. C++ v6 kept the flag meaningful on the unified index:
check_reverse_complement = false runs the ordinary single-probe lookup
and reports not-found when the match was in backward orientation —
restricting answers to k-mers occurring in forward orientation in the
indexed strings. This also coincides exactly with what the flag meant
on the removed regular mode, so pre-port strand-specific users keep
their semantics. (The old canonical mode documented the flag as
ignored; it is now honored, which is what the name promises.)
- Dictionary::{query_forward, lookup_forward, query_checked,
lookup_checked, lookup_forward_with_orientation} implement the
orientation filter instead of aliasing the default query.
- CLI --forward-only un-deprecated (real help text, no warning); the
--streaming incompatibility error stays.
- sshash-py forward_only= un-deprecated: docstrings describe the
restriction, no DeprecationWarning (BuildConfig.canonical keeps its
warning — that one is genuinely dead).
- Unit test asserts fwd-found/rc-filtered through every entry point;
pytest test_forward_only_restricts_orientation does the same from
python and asserts no warnings. CLI smoke: fwd k-mer Found:1 both
ways; rc k-mer Found:1 default, Found:0 with --forward-only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
…ADME fixes - bump_and_publish.sh gains --allow-same-version: the workspace was bumped to 0.7.0 ahead of release so downstream [patch.crates-io] path overrides resolve during development, which the script's same-version guard would otherwise reject; the flag skips the bump edits and bump commit but still runs check/publish/tag/push. - sshash-cli reported a hardcoded 0.1.0 for --version; now env!(CARGO_PKG_VERSION). - README: drop the --canonical build example (always canonical now) and the regular/canonical wording in the correctness notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
- rust-version was declared 1.85 but tiny-dict has used let-chains (stable 1.88) since the prefilter work; cargo publish never checks MSRV so this shipped silently. Workspace floor is now 1.88 (verified: sshash-lib alone still compiles on 1.85; full workspace clean on 1.88). Downstream salmon's MSRV is 1.91, so nothing tightens there. - cargo doc --no-deps is now warning-free: six pre-existing unresolved intra-doc links fixed (Self:: qualification, a renamed helper, and an extras[i] parsed as a link). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu
The rust-version bump to 1.88 enabled clippy's let-chain collapsing and is_multiple_of lints; apply them (dictionary cache block, external-sort merge loop, config validation, CLI progress check). No behavior change; clippy -D warnings clean again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NknHxdevZi243yQ5cSaffu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports C++ SSHash v6.0.0 (jermp/sshash#93): the minimizer becomes the locus minimizing h(canonical m-mer), with the centre-closest tie-break — mirror-equivariant AND forward, giving canonical answers at plain forward density. One build path, one lookup path, one streaming query (with the v6 same-minimizer memos). Index format (4,0)→(5,0); the header now records the build seed and a hasher-drift guard.
--canonicalis accepted for one release and warns; the forward-only query family keeps C++ v6 semantics (restrict answers to forward-orientation occurrences).Also fixes two latent bugs: K≥33 streaming produced wrong results in release builds (u64 truncation of u128 k-mer state — the analogue of C++ 87cab5f, proven wrong against its own point lookups), and non-default build seeds produced indices that returned zero hits (queries hard-coded seed 1).
Validation (details in commit messages and
scripts/):Perf vs 0.6.4 canonical: streaming −33%, build −12%, index bytes −8..13%, point lookups 77.7 vs 79.1 ns/kmer (an initial regression was branch misprediction from the tie tracking; fixed branchlessly).
Release order: this publishes first (
./bump_and_publish.sh 0.7.0 --publish --allow-same-version), then piscem-rs 0.10 → piscem 0.23 / salmon → simpleaf 0.29.🤖 Generated with Claude Code
https://claude.ai/code/session_01NknHxdevZi243yQ5sKaffu