Skip to content

Commit 07b560f

Browse files
refactor(rust): take the signal-level k-mer path from escapepod-signal (#222)
* refactor(rust): take the signal-level k-mer encoding from escapepod-signal leech held the only copy of this rule, inside a `crate-type = ["cdylib"]` Python extension module — so a native runtime for a leech `signal_kmer` model could not link it and had to transcribe it, which is a second definition that diverges silently. That is not hypothetical in this stack: `extract_levels` was written twice with different centring conventions and moved 25 of 100 features, and escapepod-classify reproduced a superseded feature definition for two months, its golden missing it because all 19 fixture reads took the other branch. It is also the natural pair to `escapepod_signal::mapping`, which *produces* the base-to-signal map the encoding consumes. The producing half was already upstream and the consuming half was not. escapepod-rs#272 fixed that; this is leech taking the call (escapepod-signal v0.16.0). Two primitives delegate, both byte-identical to what was here: - `encoding.rs::encode_signal_kmer_inner` -> `seq_encoding::encode_signal_kmer` - `features.rs::sequence_to_int` -> `seq_encoding::sequence_to_int`, the same A/C/G/T-U table with the same `-1` for anything else (upstream's `UNKNOWN_BASE` is `-1`) A third overlap is deliberately NOT delegated. The k-mer context slice in `signal_mapping.rs` covers the same range with the same padding as upstream's `sequence_ints_with_context`, but leech needs it as BASES — it serializes `sequence_with_kmer_context` as a string — where upstream returns ints. Same rule, different type; swapping it would change a serialized chunk field. This is a delegation, so identity is the only acceptable outcome, and the evidence is that nothing had to change to accommodate it: 198 parity tests pass untouched, including `test_backend_parity.py`, which compares every array in the npz between the Rust and Python backends — and the Python reference is not touched by this commit, so it is an independent implementation. Also here, because the pin moves anyway: - `escapepod>=0.16.0`, and the full suite re-run against that Python package as well as against the Rust crate (they are independent: leech_core links the crate statically, `leech.io` imports the wheel). 1467 passed on both. - `uv.lock` gains onnx/onnxruntime/onnxscript/protobuf. #220 added the `onnx` extra to pyproject and never locked it — CI installs from pyproject so it worked, but `uv sync --extra onnx` would not have. - The ONNX contract stops telling consumers to reimplement the encoder and names the crate that owns it. * refactor(rust): take the k-mer context windowing from escapepod-signal too The third and last overlap, and the one that needed an upstream change first (escapepod-rs#274, released in 0.16.1). It could not delegate before: leech needs the context window as BASES, because the corpus serializes `sequence_with_kmer_context` as a string, where upstream only offered ints. escapepod-signal now exposes both forms over one windowing rule, with `sequence_to_int(bases) == ints` pinned by a test there. So all three halves of the signal-level k-mer path live upstream now: `mapping` produces the base-to-signal map, `sequence_bases_with_context` cuts the window it covers plus the k-mer context, and `encode_signal_kmer` scatters the one-hot context along the signal axis. leech held the middle and the last of those, inside a cdylib no Rust consumer could link. This is the one of the three most worth getting out of a second copy. It is where `before` and `after` are not interchangeable — swap them and every k-mer is read from a window displaced by `before - after` bases, silently, and `encode_signal_kmer` cannot detect it because it only sees the total width. Identical by construction and by test: the ranges agree (`(seq_start - before)..(seq_end + after)` is upstream's `core_start - before` plus `n_bases + before + after` with `n_bases = seq_end - seq_start`), and both pad rather than shift, leech with `b'N'` and upstream with `UNKNOWN_BASE_CHAR`, which is `b'N'`. 198 parity tests pass untouched, and `sequence_with_kmer_context` is one of the fields `test_backend_parity.py` compares array-by-array between the Rust and Python backends — the Python side is not touched here, so it is an independent check. Only `rust/Cargo.toml`'s git tag moves to v0.16.1; `Cargo.lock` pins it at b4c9afae. The `escapepod` Python pin stays `>=0.16.0`, because the new function is in the Rust crate and not in the Python bindings, and the two are independent — leech_core links the crate statically, `leech.io` imports the wheel. Full suite 1467 passed, 44 skipped.
1 parent 1286c0b commit 07b560f

10 files changed

Lines changed: 295 additions & 87 deletions

File tree

CHANGELOG.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- **The signal-level k-mer encoding comes from escapepod-signal** rather than
13+
being held here (escapepod-rs#271 / #272; requires escapepod 0.16.0).
14+
`rust/src/encoding.rs` and `sequence_to_int` are now calls into
15+
`escapepod_signal::seq_encoding`.
16+
17+
leech held the only copy of this rule, inside a `crate-type = ["cdylib"]`
18+
Python extension module — so a native runtime for a leech `signal_kmer` model
19+
could not link it and had to transcribe it, which is a second definition that
20+
diverges silently. It is also the natural pair to `escapepod_signal::mapping`,
21+
which *produces* the base-to-signal map the encoding consumes: the producing
22+
half was already upstream and the consuming half was not.
23+
24+
This is a delegation, so the only acceptable outcome is identity: 198 parity
25+
tests pass unchanged, including `test_backend_parity.py`, which compares every
26+
array in the npz between the Rust and Python backends against a Python
27+
reference this change does not touch.
28+
29+
The k-mer *context slice* delegates too, via `sequence_bases_with_context`
30+
(escapepod-rs#274, escapepod 0.16.1). It could not at first: leech needs the
31+
window as **bases**, since the corpus serializes `sequence_with_kmer_context`
32+
as a string, where upstream only offered ints. Upstream now exposes both forms
33+
over one windowing rule, with `sequence_to_int(bases) == ints` pinned by a
34+
test there — so the three halves of the signal-level k-mer path (the map, the
35+
window, the encoding) all live in `escapepod-signal` and none is duplicated
36+
here.
37+
38+
That third one is the highest-stakes of the three: it is where `before` and
39+
`after` are not interchangeable, and swapping them displaces every k-mer
40+
silently because the encoder only sees the total width. It is also the most
41+
directly checkable — `sequence_with_kmer_context` is one of the fields
42+
`test_backend_parity.py` compares array-by-array between backends.
43+
44+
Only `rust/Cargo.toml`'s git tag moves to v0.16.1; the `escapepod` Python pin
45+
stays `>=0.16.0`, because the new function is in the Rust crate and not in the
46+
Python bindings.
47+
1048
### Added
1149

1250
- **ONNX export, for the classifier arms and the CRF encoder** (#217).

CLAUDE.md

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,25 @@ escapepod reads as one DP pass.
344344
Since escapepod v0.15.0 the settings themselves are escapepod's
345345
`RefineSettings::move_table_refinement` preset (escapepod-rs#257), so there is
346346
no longer a literal here to drift. **Do not hand-build `RefineSettings` in this
347-
crate again** — that is what the preset exists to prevent. Four primitives now
347+
crate again** — that is what the preset exists to prevent. Five primitives now
348348
come from upstream rather than being held locally: the preset, the POD5 reader
349349
cache (`escapepod_signal::cached_reader`), per-base statistics
350350
(`features::span_stats`, configured `SpanFill::Zero` / `SpanBounds::Clamp` /
351-
`MedianConvention::SortPartialCmp`), and the move-table and CIGAR coordinate
352-
mapping (`escapepod_signal::mapping`).
351+
`MedianConvention::SortPartialCmp`), the move-table and CIGAR coordinate mapping
352+
(`escapepod_signal::mapping`), and the whole signal-level k-mer path —
353+
the base alphabet, the context windowing and the encoding itself
354+
(`escapepod_signal::seq_encoding`, escapepod-rs#272 and #274).
355+
356+
**All three halves of that path are now upstream, and that is the point.**
357+
`mapping` produces the base-to-signal map, `sequence_bases_with_context` cuts
358+
the window it covers plus the k-mer context, and `encode_signal_kmer` scatters
359+
the one-hot context along the signal axis. leech held the middle and the last
360+
for a while, inside a `cdylib` no Rust consumer could link. The windowing is the
361+
one to watch: `before` and `after` are **not** interchangeable there, swapping
362+
them displaces every k-mer, and the encoder cannot detect it because it only
363+
sees the total width. leech keeps the window as *bases* rather than ints because
364+
the corpus serializes `sequence_with_kmer_context` as a string —
365+
`sequence_ints_with_context` is the same window in the other alphabet.
353366

354367
### The corpus is written and merged in exactly one place each
355368

@@ -529,20 +542,14 @@ the graph. The contract names the function and its parameters rather than
529542
leaving them to be re-derived — the same choice escapepod-rs's charging bundle
530543
makes by carrying its recipe in `metadata.json`.
531544

532-
**But "one definition" is not yet true, and the fix is upstream.** `leech-core`
533-
ships the encoder, and it is tempting to say a consumer should just call it —
534-
except `leech_core` is `crate-type = ["cdylib"]`, a Python extension module, so
535-
escapepod-rs cannot link it. The primitive itself
536-
(`rust/src/encoding.rs::encode_signal_kmer_inner`) is pure, dependency-free and
537-
carries no model vocabulary: sequence ints, a base-to-signal map, a signal
538-
length and a k-mer context in, a `(4 * kmer_len, signal_len)` scatter out. That
539-
is an `escapepod-signal` primitive by every rule this stack already applies —
540-
and escapepod-signal owns `mapping`, which *produces* the map this consumes, so
541-
today the producer is upstream and the consumer is downstream. Moving it is
542-
rnabioco/escapepod-rs#271. Until it does,
543-
any Rust consumer has to transcribe it, which is a second definition, and the
544-
mitigation is a cross-language golden of the kind the CRF decode and the
545-
charging features already have.
545+
**And the rule has exactly one definition, upstream.** It briefly did not:
546+
`leech_core` held the only copy, inside a `crate-type = ["cdylib"]` Python
547+
extension module that Rust cannot link, so a native runtime had to transcribe
548+
it. `escapepod_signal::seq_encoding` owns it now (escapepod-rs#271 / #272), and
549+
`rust/src/encoding.rs` is a call into it — the natural pairing, since
550+
`escapepod_signal::mapping` *produces* the base-to-signal map this consumes.
551+
A Rust consumer links the crate; a Python one calls `_rs_encode_signal_kmer`,
552+
which is the same code. **Do not reimplement it on either side.**
546553

547554
**Verification crosses the serialization boundary.** `verify_onnx` runs
548555
onnxruntime against torch and returns the max absolute difference, which is what

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ dependencies = [
2525
"click>=8.1.0",
2626
# POD5 I/O and signal normalization. Imported unconditionally by
2727
# leech.io.pod5_reader and leech.features, so it cannot be optional.
28-
"escapepod>=0.15.0",
28+
"escapepod>=0.16.0",
2929
"numpy>=2.0.0",
3030
"nvitop>=1.6.1",
3131
"polars>=1.43.2",

rust/Cargo.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ pyo3 = { version = "0.28", features = ["extension-module", "abi3-py312"] }
2828
numpy = "0.28"
2929
# Not on crates.io; pulled straight from the public GitHub repo. Pin by tag so
3030
# the build is reproducible — bump the tag to take a new escapepod release.
31-
escapepod-signal = { git = "https://github.com/rnabioco/escapepod-rs", tag = "v0.15.0" }
31+
escapepod-signal = { git = "https://github.com/rnabioco/escapepod-rs", tag = "v0.16.1" }
3232
rayon = "1"
3333

3434
[profile.release]

rust/src/encoding.rs

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,43 +2,31 @@ use numpy::ndarray::Array2;
22
use numpy::{IntoPyArray, PyArray2, PyReadonlyArray1};
33
use pyo3::prelude::*;
44

5-
/// Internal signal-level kmer encoding. Returns flat row-major Vec of shape
6-
/// `(4 * kmer_len, signal_len)`.
5+
/// Signal-level k-mer encoding, from `escapepod-signal`.
6+
///
7+
/// The rule used to live here, and it was the only copy — inside a `cdylib`
8+
/// that Rust cannot link, so any native runtime for a leech `signal_kmer` model
9+
/// had to transcribe it (rnabioco/escapepod-rs#271). It is the natural pair to
10+
/// `escapepod_signal::mapping`, which *produces* the base-to-signal map this
11+
/// consumes, so upstream now owns both halves and this is a call.
12+
///
13+
/// Returns a flat row-major `Vec` of shape `(4 * kmer_len, signal_len)`.
714
pub(crate) fn encode_signal_kmer_inner(
815
seq_ints: &[i8],
916
sig_map: &[i64],
1017
signal_len: usize,
1118
kmer_before: usize,
1219
kmer_after: usize,
1320
) -> Vec<f32> {
14-
let kmer_len = kmer_before + 1 + kmer_after;
15-
let seq_len = sig_map.len().saturating_sub(1);
16-
let enc_dim = 4 * kmer_len;
17-
let mut enc = vec![0.0f32; enc_dim * signal_len];
18-
19-
for kmer_pos in 0..kmer_len {
20-
let offset = 4 * kmer_pos;
21-
for seq_pos in 0..seq_len {
22-
let base_idx = seq_pos + kmer_pos;
23-
if base_idx >= seq_ints.len() {
24-
continue;
25-
}
26-
let base = seq_ints[base_idx];
27-
if base < 0 {
28-
continue;
29-
}
30-
let sig_start = (sig_map[seq_pos] as usize).min(signal_len);
31-
let sig_end = (sig_map[seq_pos + 1] as usize).min(signal_len);
32-
if sig_start < sig_end {
33-
let row = offset + base as usize;
34-
for s in sig_start..sig_end {
35-
enc[row * signal_len + s] = 1.0;
36-
}
37-
}
38-
}
39-
}
40-
41-
enc
21+
escapepod_signal::seq_encoding::encode_signal_kmer(
22+
seq_ints,
23+
sig_map,
24+
signal_len,
25+
escapepod_signal::seq_encoding::KmerContext {
26+
before: kmer_before,
27+
after: kmer_after,
28+
},
29+
)
4230
}
4331

4432
/// PyO3 wrapper for signal-level kmer encoding.

rust/src/inference_pipeline/features.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,12 @@ pub(super) fn compute_signal_residual(
9898
residual
9999
}
100100

101+
/// Bases as `0..3`, anything else `-1`. From `escapepod-signal`, which owns
102+
/// the alphabet the signal-level encodings are written in; the table here was
103+
/// identical, and two copies of an alphabet is how a `U` starts meaning
104+
/// something different on one side.
101105
pub(super) fn sequence_to_int(sequence: &[u8]) -> Vec<i8> {
102-
sequence
103-
.iter()
104-
.map(|&c| match c {
105-
b'A' | b'a' => 0,
106-
b'C' | b'c' => 1,
107-
b'G' | b'g' => 2,
108-
b'T' | b't' | b'U' | b'u' => 3,
109-
_ => -1,
110-
})
111-
.collect()
106+
escapepod_signal::seq_encoding::sequence_to_int(sequence)
112107
}
113108

114109
pub(super) fn encode_base_onehot(sequence: &[u8]) -> Vec<f32> {

rust/src/inference_pipeline/signal_mapping.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,25 @@ pub(super) fn chunk_signal_kmer_inputs(
112112
map[0] = 0;
113113
map[last] = chunk_len as i64;
114114

115+
// The windowing comes from escapepod-signal, which also owns the map this
116+
// sits between and the encoding it feeds. Kept as BASES rather than ints
117+
// because the corpus serializes `sequence_with_kmer_context` as a string;
118+
// `sequence_ints_with_context` is the same window in the other alphabet,
119+
// and `sequence_to_int` of this is exactly that (escapepod-rs#274).
120+
//
121+
// This is the step where `before` and `after` are NOT interchangeable:
122+
// swapping them displaces every k-mer silently, and the encoder cannot
123+
// detect it because it only sees the total width.
115124
let (kmer_before, kmer_after) = skmer_ctx;
116-
let ctx_lo = seq_start as i64 - kmer_before as i64;
117-
let ctx_hi = seq_end as i64 + kmer_after as i64;
118-
let ctx: Vec<u8> = (ctx_lo..ctx_hi)
119-
.map(|i| match usize::try_from(i) {
120-
Ok(u) if u < seq_bytes.len() => seq_bytes[u],
121-
_ => b'N',
122-
})
123-
.collect();
125+
let ctx = escapepod_signal::seq_encoding::sequence_bases_with_context(
126+
seq_bytes,
127+
seq_start,
128+
seq_end - seq_start,
129+
escapepod_signal::seq_encoding::KmerContext {
130+
before: kmer_before,
131+
after: kmer_after,
132+
},
133+
);
124134

125135
(map, ctx)
126136
}

src/leech/onnx_export.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,13 @@ def describe_inputs(config: dict, example_inputs: tuple) -> list[InputSpec]:
8787
"leech.features.encode_signal_kmer(signal, seq_to_sig_map, sequence, "
8888
f"context={list(kmer_context)}) — a scatter of the one-hot k-mer context "
8989
"along the signal axis. It lives in the dataset, not the model, so it is "
90-
"NOT in this graph. Python consumers should call leech-core's "
91-
"`_rs_encode_signal_kmer`. A non-Python consumer currently has to "
92-
"reimplement it — leech_core is a cdylib and cannot be linked from Rust — "
93-
"so pin any reimplementation against a golden rather than trusting it; "
94-
"two definitions of this rule diverge silently."
90+
"NOT in this graph — a consumer must produce it before calling the "
91+
"model. The rule is owned by escapepod-signal "
92+
"(`escapepod_signal::seq_encoding::encode_signal_kmer`), which leech "
93+
"calls rather than duplicates; a Rust consumer should link that crate, "
94+
"and a Python one can call leech-core's `_rs_encode_signal_kmer`, which "
95+
"is the same code. Do not reimplement it — two definitions of this rule "
96+
"diverge silently."
9597
)
9698
else:
9799
produced_by = "one-hot encoding of the k-mer context, 4 channels"

0 commit comments

Comments
 (0)