perf(streaming): fold the mixed record decode into the producer (#400) - #420
Merged
Merged
Conversation
…ucer (#400) Removes the second VCF/PGEN window decode on the mixed variants+tracks path: read the window the producer already decoded from the drive engine's current slot, via a new with_current_window core primitive and a current_window_realign_inputs pymethod that validates window identity against the engine's own job. Deletes the per-backend _mixed_engine() second engine; keeps window_realign_inputs as a documented test-only oracle for the CSR replication test. SVAR1 deliberately not folded (offsets-only read). Gate is deterministic decode counters, not wall-clock.
Four TDD tasks: with_current_window in the engine core, the current_window_realign_inputs pymethod, the Python fold plus the deterministic-counter gate, then doc/roadmap retirement and the full-tree sweep.
with_current_window needs the current window's slot WITHOUT consuming a row, which advance's fused check-and-advance cannot express. Extract the ensure half and reimplement advance on top of it; control flow is unchanged, so every existing advance caller (next_batch_core, next_batch_variants_core, ...) keeps its exact behaviour. Rust unit test pins the peek semantics on a stub backend.
RecordStreamEngine::current_window_realign_inputs reads the producer's already-decoded slot via the new with_current_window core primitive, so the Python drive can size a window's track query without a second decode. The requested job is validated against the engine's current job: a drifted plan raises ValueError instead of pairing one window's tracks with another's variants.
The new test's docstring claimed window_realign_inputs was already test-only; at this commit _mixed_engine still drives it. Reword to the future tense so the claim lands only once this branch's fold deletes that caller. Also pin the exhausted-plan path: a plan-less engine has no current window and must return None.
The VCF/PGEN mixed variants+tracks path decoded every window twice: the producer filled it for the haplotypes, then the track side re-decoded it synchronously through a private plan-less engine. The drive's engine is now the only decoder: _mixed_track_window forwards it and _record_mixed_realign_window reads the producer's current slot via current_window_realign_inputs, so realign_tracks=True costs the same decode as realign_tracks=False (gated on transpose_word_reads and pgen_variants_decoded, not wall-clock). The private engines are deleted.
window_realign_inputs and debug_decode_window are now test-only seams: the record mixed path reads the producer's own slot. Update the three Rust doc comments that still describe the double decode, and tick the roadmap's #400 follow-ups with the measured decode-counter numbers.
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.
Closes #400.
The VCF/PGEN mixed variants+tracks path decoded every window twice: the Rust
producer filled it for the haplotypes, then the track side re-decoded it
synchronously through a private plan-less
_mixed_engine(). The drive'sRecordStreamEngineis now the only decoder.StreamEngineCore::with_current_window+ensure_current_window(split out ofadvance): read the current window's slot without consuming a row.RecordStreamEngine.current_window_realign_inputs: returns that window's(v_starts, ilens, geno_v_idxs, geno_offsets), validating the requested jobagainst the engine's current job so a drifted plan raises
ValueErrorinsteadof pairing one window's tracks with another's variants.
enginethreaded through_mixed_track_windowand_RecordStreamEngine-backedmixed_realign_window; the private plan-lessengines are deleted. SVAR1/SVAR2 accept and ignore the parameter.
Gate is deterministic, not wall-clock:
transpose_word_reads(and PGEN'spgen_variants_decoded) are now identical withrealign_trackson and off,where the realign run was ~2x before. Every existing mixed parity suite is
unchanged and green. No public API change;
window_realign_inputsstays as thetest-only oracle.