Skip to content

feat!: report match locations as SeqId with O(1) header lookup#9

Merged
sriram98v merged 3 commits into
mainfrom
feat/seqid-header-split
Jul 24, 2026
Merged

feat!: report match locations as SeqId with O(1) header lookup#9
sriram98v merged 3 commits into
mainfrom
feat/seqid-header-split

Conversation

@sriram98v

Copy link
Copy Markdown
Owner

Summary

Queries now identify a matching reference by a stable integer id rather than by its header string, and the header ↔ id mapping becomes a separate, O(1) concern.

Previously the locate path allocated one String per occurrence: it resolved a position to (seq_idx, pos) and then immediately cloned seq_headers[seq_idx], only for callers to hash it back to an integer. That cost is per occurrence, not per seed, so it scales with seed multiplicity — hundreds of references for a conserved seed in a metagenomic database, multiplied again by read orientations and pairs.

The integer already existed as the index into the build-ordered header list, and the GPU MEM path already reported it directly (MemHit::positions). This exposes it on the CPU path too and makes both paths agree on one representation.

Addresses the feature request filed from PREMISE (integer reference IDs from locate).

Breaking changes

  • SeqId(u32) is the new id type. A newtype rather than a bare u32, so an id can never be swapped with the offset it is paired with — both are u32, and (SeqId, u32) tuples would otherwise be trivial to transpose. Assigned in build order, preserved across to_bytes/from_bytes, with new / get / index accessors.
  • Queries return (SeqId, u32) instead of (String, u32)FmIndex::locate, FmIndex::locate_gpu, FmIndex::map_position, BidirFmIndex::locate_interval, Mem::positions, and the WASM locate. MemHit::positions moves from (u32, u32) to (SeqId, u32), so CPU and GPU MEM results stay the same shape.
  • Headers must now be unique. A collision raises the new FmIndexError::DuplicateHeader at build time — this is what lets seq_id be an exact inverse of seq_header rather than a first-wins approximation. It is checked before suffix-array construction, so a bad reference set fails in milliseconds instead of after the expensive build. Sequences supplied without a header are still auto-named seq_{i}, so this only fires on genuinely repeated names.

Migration: callers that need labels resolve ids through seq_header(id), or build an id -> label table once from seq_headers() and index it by SeqId::index().

Added

  • seq_headers(), seq_header(id) and seq_id(header) on both FmIndex and BidirFmIndex. Both directions are O(1): seq_header indexes the header vec, seq_id uses a HashMap<Box<str>, u32> built at construction. The map is derived from the headers, so it is not serializedfrom_bytes rebuilds it, leaving the index format unchanged.
  • WASM bindings seq_header, seq_id, seq_headers (wasm-bindgen preserves the Rust snake_case, matching the existing text_len / num_sequences).

Also

  • The web demo was rendering locate() pairs as if they were a flat position list, printing "seq_0,3" in a single column — already wrong before this change. It now resolves ids through seq_headers() once and shows Sequence / Position columns, with HTML escaping since headers come from user-pasted FASTA.
  • docs/ (mdBook) previously used the term seq_id for both concepts on the same page — "the sequence's name (a String)" in one section and "numeric (seq_id, offset) indices" in another. A new Sequence ids vs. headers section in concepts.md defines both terms and is cross-linked from five pages.

Test plan

  • cargo fmt --check
  • cargo clippy --all-features -- -D warnings — 0 warnings (verified with a forced full re-lint)
  • cargo build --all-features
  • cargo test — 15 binaries, 164 tests, 0 failed
  • cargo test --features gpu — all green against a real adapter, including the GPU/CPU MEM parity suites
  • cargo doc --no-deps, cpu-only and --all-features — no warnings
  • Full deploy.yml dry run: wasm32 release build → wasm-bindgennpm civite buildmdbook build → site assembly
  • npx tsc --noEmit on the demo — no errors in main.ts, typechecked against freshly generated bindings

New tests/seq_ids.rs validates located ids against a brute-force search over the same corpus rather than against another index code path, so it stands on its own:

  • Parity at SA sampling rates 1/2/4/8 — rate ≥ 2 exercises the LF-walk that multi-sentinel indexes have previously got wrong, and the old locate tests only covered rate 1.
  • seq_id / seq_header are exact inverses; out-of-range ids and unknown headers return None.
  • Duplicate-header rejection, including the subtle case of an explicit header colliding with a generated seq_{i} name.
  • Ids stable across to_bytes/from_bytes, and header lookup still works after deserialization (the map is rebuilt, not stored).

A seq_id/seq_header inverse property was added to proptest_correctness.rs, and gpu_mem_positions_parity.rs lost the "seq_N"N string parsing it previously needed to compare CPU against GPU — both sides are SeqId now.

Notes for review

  • Cargo.lock is gitignored, so CI resolves dependencies fresh while local runs used this checkout's lock (wasm-bindgen 0.2.117). Pre-existing and unrelated to this change, but it means CI exercises newer minor versions than were tested locally.
  • The three commits are kept separate deliberately: 408c553 is additive, 825e247 is the breaking change, 3b3c709 is docs-only. Squash if you prefer a single entry.

sriram98v added 3 commits July 24, 2026 14:50
The CPU locate path allocated one String per occurrence: FmIndex::locate and
BidirFmIndex::locate_interval resolved a position to (seq_idx, pos) and then
immediately cloned seq_headers[seq_idx], only for callers to hash it back to an
integer. That cost is per occurrence, not per seed, so it scales with seed
multiplicity — hundreds of references for a conserved seed in a metagenomic
database, multiplied again by read orientations and pairs.

The integer already existed as the index into the build-ordered seq_headers, and
the GPU MEM path already reported it directly (MemHit.positions). Expose it on
the CPU path too.

Added, all additive — existing String-returning APIs keep their signatures and
behavior:

- Accessors on FmIndex and BidirFmIndex: seq_headers(), seq_header(id),
  seq_id(header). Ids are 0-based in build order and stable across
  serialization, so callers can build an `id -> label` table once at load time
  instead of hashing a header per hit. seq_id is an O(n) scan, documented as
  such, with seq_headers() as the bulk path.
- FmIndex::locate_ids and BidirFmIndex::locate_interval_ids, returning
  (seq_id, pos). The String variants are now thin mapping layers over these, so
  there is a single implementation per path.
- BidirFmIndex::find_smems_ids / find_mems_ids returning the new MemIds type.
  MemIds::positions has the same (seq_id, offset) shape as MemHit::positions, so
  CPU and GPU MEM results can be consumed by the same code. The SMEM/MEM search
  itself is unchanged — the candidate-collection stages were extracted into
  smem_raws/mem_raws and are shared by both the String and id variants.
- WASM bindings: locateIds, seqHeader, seqId.

No index format change: seq_id is the position in the already-serialized
seq_headers, so nothing new is stored and no version bump is needed.

Tests: new tests/id_locate_parity.rs asserts the _ids variants report exactly
the same occurrences as their String counterparts, across SA sampling rates 1-8
(rate >= 2 exercises the LF-walk that multi-sentinel indexes have previously got
wrong) and across a to_bytes/from_bytes round trip, plus accessor round-trip and
rejection cases. A matching proptest property was added to
proptest_correctness.rs.
…ways

Queries now identify a matching reference by a stable integer id rather than by
its header string, and the header <-> id mapping is a separate, O(1) concern.

Previously the locate path allocated one String per occurrence: it resolved a
position to (seq_idx, pos) and then immediately cloned seq_headers[seq_idx], only
for callers to hash it back to an integer. The cost was per occurrence rather
than per seed, so it scaled with seed multiplicity — hundreds of references for a
conserved seed in a metagenomic database, multiplied again by read orientations
and pairs.

BREAKING CHANGES

- `SeqId(u32)` is the new id type — a newtype so an id can never be swapped with
  the offset it is paired with, since both are u32. Assigned in build order,
  preserved across to_bytes/from_bytes, with `new`/`get`/`index` accessors.
- Queries return `(SeqId, u32)` instead of `(String, u32)`: `FmIndex::locate`,
  `FmIndex::locate_gpu`, `FmIndex::map_position`, `BidirFmIndex::locate_interval`,
  `Mem::positions`, and the WASM `locate`. `MemHit::positions` moves from
  `(u32, u32)` to `(SeqId, u32)`, so CPU and GPU MEM results stay the same shape.
- Headers must now be unique. A collision raises the new
  `FmIndexError::DuplicateHeader` at build time, which is what lets `seq_id` be an
  exact inverse of `seq_header`. It is checked before suffix-array construction so
  it fails fast. Sequences supplied without a header are still auto-named
  `seq_{i}`, so this only fires on genuinely repeated names.

Added

- `seq_headers()`, `seq_header(id)` and `seq_id(header)` on both `FmIndex` and
  `BidirFmIndex`. Both directions are O(1): `seq_header` indexes the header vec,
  and `seq_id` uses a `HashMap<Box<str>, u32>` built at construction. The map is
  derived from the headers, so it is not serialized — `from_bytes` rebuilds it,
  leaving the index format unchanged.
- WASM bindings `seq_header`, `seq_id`, `seq_headers` (wasm-bindgen preserves the
  Rust snake_case names, matching the existing `text_len` / `num_sequences`).

Also

- The web demo was rendering locate() pairs as if they were a flat position list,
  printing "seq_0,3" in a single column. It now resolves ids through
  `seq_headers()` once and shows Sequence / Position columns, with HTML escaping
  since headers come from user-pasted FASTA.

Tests: tests/seq_ids.rs (replacing tests/id_locate_parity.rs) checks located ids
against a brute-force search over the same corpus rather than against another
index code path — across SA sampling rates 1-8, since rate >= 2 exercises the
LF-walk that multi-sentinel indexes have previously got wrong. It also covers the
accessor inverse property, duplicate-header rejection (including an explicit
header colliding with a generated `seq_{i}` name), and that header lookup still
works after deserialization. A `seq_id`/`seq_header` inverse property was added to
proptest_correctness.rs, and gpu_mem_positions_parity.rs lost the "seq_N" -> N
parsing it needed to compare CPU against GPU.
The mdBook site still described the old API, and its wording actively conflated
the two concepts: count-locate.md called the locate result's `seq_id` "the
sequence's name (a String)", while the GPU section on the same page used the same
term for the integer index.

- concepts.md gains a "Sequence ids vs. headers" section defining both terms,
  explaining why queries return the id (locate cost is per occurrence, so cloning
  a header per hit scales with seed multiplicity), showing the O(1) conversions,
  and stating the stability guarantee and the uniqueness requirement.
- count-locate.md: corrected return type, a dedicated ids-and-headers section
  with both the per-hit and build-a-table-once patterns, and a note that the GPU
  path reports the same shape.
- quick-start.md, mem-smem.md (Mem and MemHit), wasm.md, api.md: corrected
  signatures, plus the JS accessors and a pointer to the concepts section.
- serialization.md: ids are stable across a round trip and may be recorded
  against a serialized index, but are only meaningful within one index.

Also fixes two comments of mine that used camelCase JS names — wasm-bindgen
preserves the Rust snake_case, so the bindings are seq_header / seq_id /
seq_headers.

Verified with `mdbook build`: the book builds and all five cross-links resolve to
the generated `#sequence-ids-vs-headers` anchor.
@sriram98v
sriram98v merged commit 099a68e into main Jul 24, 2026
4 checks passed
@sriram98v
sriram98v deleted the feat/seqid-header-split branch July 24, 2026 21:46
sriram98v added a commit that referenced this pull request Jul 24, 2026
0.1.0 is published on crates.io (2026-07-16, not yanked), so the SeqId change
in #9 broke a released API without a version bump. Pre-1.0, the minor version
is the breaking-change slot, so this is 0.2.0.

Also corrects the changelog, which still filed the released 0.1.0 content under
[Unreleased]. Split into [0.1.0] - 2026-07-16 (what actually shipped) and
[0.2.0] - 2026-07-24 (the SeqId work), with the two breaking items called out
explicitly under Changed rather than left implicit in the version number.

Serialization compatibility, verified against an index written by 0.1.0 itself
(built at a81dc45 in a scratch worktree) rather than assumed:

- unique headers: loads under 0.2.0, header map rebuilt correctly, locate works
- duplicate headers: rejected with DuplicateHeader

0.1.0 permitted header collisions, so that second case is a real compatibility
break for anyone who serialized such an index. The changelog says so and tells
them to rebuild, instead of claiming blanket format compatibility.

No tag is created here — tagging and `cargo publish` are left as a separate,
deliberate step.

Co-authored-by: sriram98v <sriram.98v@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant