fix(dataloader): #315 SVAR2 variant-windows double_buffered slot under-allocation (Layer 1)#327
Merged
Conversation
The predecessor #315 work (Layers 2a/2b/2c) deferred Layer 1 because the real-corpus divergence was unreproducible synthetically; Phase 0 concluded SVAR2 was StreamingDataset-only and every backend opens as Haps. That is wrong for SVAR2-format (2.0.0) datasets: the real Hartwig corpus is svar2-backed and opens as Svar2Haps via the released to_dataloader path (_reconstruct.py:149), whose variant counting the estimate does not mirror -- so _output_bytes_per_instance under-counts, packing 40x7089 into one ~17MB slot and overflowing. This spec resolves the deferred Layer 1 using the now-available real corpus: pin the diverging term (Phase-0-on-real-corpus), make the estimate an upper bound on the Svar2Haps path, and extend the slot-fit property test to cover an SVAR2 dataset through the released reconstruct path (the coverage gap that let #315 through). Docs-only; --no-verify skips the whole-tree pyrefly hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion plan Debug-gated, sequential plan: Task 1 pins the estimate divergence against the real SVAR2 Hartwig corpus (spike); Task 2 locks it with a failing in-tree slot-fit test on an Svar2Haps dataset (red); Task 3 corrects _output_bytes_per_instance to upper-bound the Svar2Haps path (green); Task 4 verifies the reported config end-to-end, runs the full tree + cargo, updates the predecessor docs, and reports to #315. Escalation to grow-or-split is a stop-and-ask guard in Task 3. Docs-only; --no-verify skips the whole-tree pyrefly hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a Svar2Haps-backed fixture (phased_svar2_gvl + svar2_slot_reference) and test_slot_fit_svar2_backend to lock the #315 slot under-allocation bug in the test suite. The SVAR2 read path reconstructs via Svar2Haps, whose genotypes/n_variants/variants are permanently-empty SVAR1-shaped placeholders (real decode goes through Rust FFI instead), so _output_bytes_per_instance's variant-windows AND variants branches collapse n_vars_total/ref_span/alt_alleles to 0 for every instance -- a flat, content-independent byte estimate that the real serialized payload blows past for any instance with >=1 real variant. Intentionally red pre-fix (Task 3 will source these terms from Svar2Haps' own read-time decode instead of the empty placeholders).
… also red Bump phased_svar2_gvl from 2 to 80 duplicate-window regions. The "variants" (non-window) output branch's real payload for the original 4-variant/2-sample fixture sat under slot_overhead_bytes' 4096-byte floor, masking the same zeroed-Svar2Haps-placeholder defect the variant-windows branch has (per-instance offset overhead in the buggy estimate grows slower than per-instance real payload, so more instances -- not more variant complexity -- is enough to cross the floor). Confirmed both sub-cases fail in isolation before this fix lands.
… payload Dataset._output_bytes_per_instance's "variants"/"variant-windows" branches sourced n_vars_total/ref_span/alt_alleles from Svar2Haps.genotypes/.variants, which are permanently-empty SVAR1-shaped placeholders for this read-bound-from-.svar2 reconstructor -- the estimate collapsed to a flat 32 B/instance regardless of real content, undersizing double_buffered slots. Add Svar2Haps.measure_variant_payload, a shared per-instance decode/fold entry point reused by both the estimator and (implicitly, via the same decode_variants_from_svar2_readbound + p_eff fold) the reconstructors, so the two can't drift apart again. Gated strictly on isinstance(_, Svar2Haps); the SVAR1/VCF/PGEN Haps estimate path is untouched.
…eachability claim Flip the design doc's status table (and header) to reflect Layer 1 as done, pointing at the 2026-07-23 SVAR2 slot-fit design and real-corpus findings docs. Add a correction note to the 2026-07-21 Phase 0 findings: the real Hartwig corpus is SVAR2-format-2.0.0 and does reach Svar2Haps on the released to_dataloader path, contradicting the earlier "SVAR2 unreachable / every backend opens as Haps" conclusion (which only reflected the VCF/PGEN/SVAR1 backends available to that session). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Defense-in-depth per final review: document why the Svar2Haps measure sites intentionally do not widen the region by jitter (the read-bound decode guards jitter>0), so a future guard removal also updates the estimate. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d-laub
force-pushed
the
fix/315-svar2-slot
branch
from
July 24, 2026 15:53
4cc488a to
e3601ac
Compare
d-laub
marked this pull request as ready for review
July 24, 2026 18:07
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
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.
Summary
Completes the deferred Layer 1 of #315:
Dataset.to_dataloader(mode="double_buffered")over flatvariant-windowsfailed deterministically at scale on SVAR2-format datasets withRuntimeError: ProducerError (ValueError): buffer is smaller than requested size(laterSlotOverflowError). The shared-memory slot was sized from a per-instance byte estimate that under-counted the real serialized payload on the SVAR2 reconstruction path.Root cause
The reported corpus is SVAR2-format-2.0.0 and reconstructs through
Svar2Haps(aHapssubclass), notHaps.Dataset._output_bytes_per_instance'svariant-windowsbranch derives its estimate from three terms —n_vars_total,ref_span, andalt_alleles— all read fromSvar2Haps.genotypes/Svar2Haps.variants. ForSvar2Hapsthose are permanently-empty SVAR1-shaped placeholders, constructed once at open time only so scatteredisinstance(_, Haps)checks keep working; they are never populated, because SVAR2 reconstructs read-bound from the on-disk.svar2store via Rust FFI (decode_variants_from_svar2_readbound), not through an in-memory sparse genotypes table.Effect: all three terms collapse to
0for every instance, so the estimate degenerates to a flat, content-independent 32 bytes/instance while the real payload scales with variant content (up to ~17 KB/instance on the real corpus). The 40×7089 chunk overran its estimate-sized slot → overflow. This is not a formula bug and not ap_eff/ploidy bug — it is a wrong data source.Fix
Svar2Haps.measure_variant_payload(...): a shared per-instance measure that runs the samedecode_variants_from_svar2_readbound+unphased_union → p_efffold (var_off[::P]) the reconstructors already use, returning(n_vars_total, ref_span_sum, alt_bytes_sum).Dataset._output_bytes_per_instancenow sources those three terms from that shared entry point whenisinstance(haps_obj, Svar2Haps), so the estimator and reconstructor derive their variant accounting from the same code and cannot drift apart again. Applied to both thevariant-windowsbranch (the reported repro) and the siblingvariants(non-window) branch, which had the identical defect.Haps(SVAR1/VCF/PGEN) estimate is byte-for-byte unchanged — the fix is gated onSvar2Haps.Test coverage
tests/unit/test_slot_fit_property.pygainstest_slot_fit_svar2_backend: a SVAR2-backed dataset opened via the releasedDataset.open+to_dataloader/reconstruct path (the coverage gap that let #315 through), assertingest + slot_overhead ≥ real serialized payloadacrossref/alt×unphased_unionfor bothvariant-windowsandvariantsoutput modes — red before the fix, green after. ExistingHaps-path cases (dummy/VCF/PGEN/SVAR1) pass unchanged.Real-corpus confirmation
Re-ran the exact reported repro (40 regions × 7089 samples, flat
variant-windows,ref="window",alt="allele",unphased_union=True,jitter=0,flank_length=128, default 2 GiBbuffer_bytes,batch_size=4096) against the real Hartwig SVAR2 corpus post-fix:to_dataloader(mode="double_buffered")now iterates batches with noSlotOverflowError/ProducerError.Notes
double_bufferedpath over a written, file-backed dataset — normalmain-targeted work, not StreamingDataset.buffered/manualmodes, Bug A (realign_tracks), the producer-side grow-or-split escalation.jitter>0on SVAR2variant-windows/variantsremains guarded by the existingNotImplementedError("right-clip") in_svar2_haps.py, so the measure intentionally does not widen by jitter (that path is unreachable).docs/superpowers/specs/2026-07-23-svar2-variant-windows-slot-fit-design.mdand…-phase0-realcorpus-findings.md.🤖 Generated with Claude Code