Skip to content

chunk format: one CHUNK_FIELDS spec, ASCII text in ChunkSpool, one BAM tag parser #288

Description

@jayhesselberth

Tier P3 · effort M · from the 2026-09-13 review (prep F7, F10, F12)

  • The chunk record's field list is spelled four times: chunking/serialization.py:349-477 (iter_chunk_columns, write), :1044-1176 (load_chunks, read as dicts), chunking/table.py:28-47 (_INT_FIELDS/_TEXT_FIELDS, read as columns), and the classifier in tests/test_backend_parity.py. serialization.py:381-382 defaults feature_start/feature_end to literal -5/5 — a third spelling of the rule resolve_feature_window centralised in Rust prepare backend ignores --feature-start: 25-base feature window instead of 20, on every chunk #189 (never hit, since both backends set them, but a trap).
  • ChunkSpool keeps every text member as <U in RAM (serialization.py:720-724): read_ids U36 (144 B), sequences_with_kmer_context ~U56 (224 B), sequences U11 (44 B) plus three small ones ≈ 450 B/chunk ≈ 3 GB at 6.7M chunks — the dominant RSS of data prepare now that arrays spill; text_column() (:748-760) then concatenates a second transient copy. table.py:144-165 already reads text as fixed-width S.
  • mv/ns/ts are parsed in io/bam_reader.py:151-157 (ReadInfo.__init__) and again in features.py:131-155 (extract_move_table); CL in bam_reader.py:160-166 and preparation/reader.py:277-281.

Acceptance criteria

  • One CHUNK_FIELDS spec (field, member, dtype, missing sentinel, is_text) consumed by the writer, the dict loader, ChunkTable and the parity test's classifier; _text() and the -1 sentinels are properties of the spec; the -5/5 literals are gone. Adding a field is one edit plus its parity classification.
  • ChunkSpool holds text as fixed-width S and widens to <U blockwise in _write_text_member; the .npz bytes are identical (tests/test_chunk_writer.py); peak RSS of data prepare on the fixture drops (record it).
  • MoveTable.from_alignment(aln) is the single tag parser; ReadInfo and extract_move_table delegate; ReadInfo.from_alignment owns CL.
  • tests/test_backend_parity.py still fails on an unclassified field.

Non-goals

  • The struct-of-arrays Rust return (separate issue), though this spec is what it should target.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestreview-2026-09Findings from the 2026-09-13 codebase review

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions