Skip to content

test: gvl.concat has no svar2 coverage — add an .svar2 concat fixture #343

Description

@d-laub

Split out from the gvl.concat work (#334, PR #339). Deferred deliberately: an .svar2 fixture needs test infrastructure the concat plan never scoped.

What is missing

gvl.concat implements the .svar2 backend (_concat_svar2_ranges in python/genvarloader/_dataset/_concat.py), but no test exercises it. The concat test suite covers PGEN/VCF on both axes, .svar on both axes, and tracks/annot tracks — but the entire svar2 branch ships untested.

The blocker is fixture infrastructure: tests/_builders/case.py's Case dataclass has pgen_path and svar_path but no svar2_path, so there is no session-scoped .svar2 store to shard and merge.

Why it was not treated as blocking

A review verified the svar2 merge code against the real on-disk format rather than against tests, and found it consistent:

  • filenames (vk_snp_range.npy, vk_indel_range.npy, dense_snp_range.npy, dense_indel_range.npy, sample_cols.npy, svar2_meta.json)
  • the raw-memmap vs np.save split — the four range arrays are written headerless via gather_fixed/link_or_copy_buffered; sample_cols goes through np.save, matching the reader's np.load
  • the 16-byte record stride and (R, S, P, 2) C-order slot space
  • the four ["shape"] updates in svar2_meta.json, with "ploidy" preserved
  • the reader derives R from dense_snp_range.shape[0] and S from vk_snp_range.shape[1], both of which the merge sets correctly

Worth noting the region-axis dense_* gather also fixes a latent block-concat bug that was present in the original plan's np.concatenate approach.

So: consistent-by-inspection, but unverified by execution.

What to do

  1. Add an .svar2 store to the shared synthetic case (svar2_path on Case, built alongside the existing .svar).
  2. Add concat fixtures sharding it by region and by sample.
  3. Assert against a single-shot gvl.write oracle, mirroring the existing svar tests.
  4. Include an interleaved sample split (e.g. [s0, s2] + [s1]). This matters: the plain fixtures split samples alphabetically, so after re-sorting they collapse to a block layout — the exact case a wrong slot ordering would also pass. Only an interleaved split proves the order= handling is right for svar2.

Deferred minors from the same review

Worth folding in if someone is already in this code:

  • svar_link/svar2_link are copied verbatim into merged metadata, but relative_path resolves against the dataset dir, so it is stale for a merged dataset written elsewhere. Resolution silently falls back to absolute_path, so tests pass — a later tree move breaks it. Recompute the link against the new destination.
  • _gather_svar_offsets has unused n_src_slots/n locals, re-reads each input's offsets file once per plane (2x IO), and np.stack(planes).tobytes() adds two full-size copies on top of the permitted materialization. Writing each plane's bytes separately would halve peak memory inside that carve-out.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions