Repository navigation
Harden RSS LD sketch chromosome merging and indel event identification - #1404
Merged
gaow merged 6 commits intoAug 19, 2026
Merged
Conversation
added 3 commits
August 18, 2026 15:50
Narrow event-ID assignment from all indels to only indels that form an exact REF/ALT-swap pair at the same position (the ambiguous cases where INS vs DEL anchoring can flip effect-allele orientation). Non-mirror indels, SNPs, and equal-length substitutions retain standard IDs. merge_chrom now substitutes the canonical event ID into .pvar and .afreq for the mirror set, and writes no .event_id.tsv when a panel has no mirror pairs (leaving .pvar/.afreq fully standard). Notebook narrative updated to match. Validated against deployed R5 EUR chr1 (reproduces 48014 mirror event IDs exactly).
danielnachun
force-pushed
the
agent/rss-ld-sketch-afreq-event-id
branch
from
August 18, 2026 22:50
de4c783 to
2738bca
Compare
added 3 commits
August 19, 2026 09:56
Test coverage for the mirror-pair event-ID feature: - process_block VCF discovery now accepts .vcf.gz in addition to .bgz (rss_ld_sketch.R); both extensions validated. - Augment the rss_ld_sketch test fixture with one same-position REF/ALT-swap indel pair (A/AT + AT/A at chr22:16500000, ~40% AF, well-called) injected into the existing 60-sample cohort; convert fixture to .vcf.gz. - Regenerate expected/afreq_deterministic.tsv (673->675 variants) and add expected/event_id.tsv (the mirror-pair mapping). - Update test_rss_ld_sketch.py: .bgz->.vcf.gz, single-dot cohort-id, 673->675, and a new test_rss_merge_chrom_mirror_event_ids asserting the INS:T/DEL:T relabeling and the .event_id.tsv sidecar. - Notebook narrative: name the .vcf.gz fixture, note both extensions accepted, show the mirror pair in the Output example, correct stale W_B50.npy -> .rds. All 5 tests in test_rss_ld_sketch.py pass.
merge_chrom conflated two responsibilities: assembling the per-chromosome pgen, and canonicalizing mirror-pair indel IDs. Separate them: - Extract canonicalize_mirror_event_ids(chrom, pos, id, ref, alt) as a pure function that computes the mirror-pair event-ID mapping from variant records and returns it (0-row data.frame if no pairs). No file I/O. - do_merge_chrom now calls it, then owns reading/writing/ID-substitution as before. Output is byte-identical (5 pipeline tests unchanged; full SoS pipeline reproduces the same 675-variant / INS:T+DEL:T result). - Guard the CLI with sys.nframe()==0L so the worker is sourceable for unit tests. - Add a direct unit test (test_canonicalize_mirror_event_ids_unit + helper R script) exercising the rule on an in-memory fixture in ~1s: mirror pair relabeled, non-mirror indel and SNP excluded, empty input -> empty mapping. Surfaced and fixed a latent empty-input edge case.
Follow-through on the canonicalization extraction: pull the remaining inline concerns out of do_merge_chrom so each step is a named, focused function and the merge step reads as a sequence of calls. - reconcile_afreq(): gather per-block .afreq, validate against .pvar, reorder, atomically install; returns pvar. - apply_event_id_substitution(): write the .event_id.tsv sidecar and substitute event IDs into .pvar/.afreq (collision- and duplicate-guarded, atomic). - summarize_block_filters(): tally per-block .meta counts and print the summary. - cleanup_merge_intermediates(): remove plink2 merge temps and block dirs. - Hoist read_tab() to a file-level helper shared by the above. do_merge_chrom drops from ~113 lines to a ~25-line orchestrator: reconcile -> canonicalize -> (substitute | note-standard) -> summarize -> cleanup. Behavior byte-identical: 6 tests pass and the full SoS pipeline reproduces the same 675-variant / INS:T+DEL:T result with identical reconcile/substitution/ filter-summary messages.
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
2 participants
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.
Adds canonical directional event IDs for same-position REF/ALT-swap ("mirror") indel pairs in the RSS LD-sketch pipeline, so insertion vs. deletion anchoring can't flip effect-allele orientation during association harmonization.
Scope
The change spans three parts of the
rss_ld_sketchmodule:merge_chrom(core feature) — detects indels that form an exact REF/ALT-swap pair at the same position and assigns each a canonical event ID (chr:pos:INS:x/chr:pos:DEL:x, DEL anchored at pos+1). The event ID replaces the variant ID in.pvarand.afreq; the original↔event mapping is written to a.event_id.tsvsidecar. Only mirror-pair indels are relabeled — SNPs, equal-length substitutions, and non-mirror indels keep standard IDs. If a panel has no mirror pairs, no sidecar is written and.pvar/.afreqstay fully standard..pgenis never modified.process_block(supporting change) — VCF shard discovery now accepts.vcf.gzin addition to.bgz. Both extensions validated end-to-end.Test fixture + coverage — the
rss_ld_sketchtest fixture now carries one same-position REF/ALT-swap indel pair (A/AT+AT/Aat chr22:16500000) injected into the existing 60-sample cohort, converted to.vcf.gz. Addsexpected/event_id.tsv, regeneratesexpected/afreq_deterministic.tsv(673→675 variants), and addstest_rss_merge_chrom_mirror_event_idsasserting the INS:T/DEL:T relabeling and the sidecar contents. All 5 tests pass.Validation
Notes for reviewers
merge_chromto include theprocess_block.vcf.gzdiscovery change and the test fixture/coverage — flagged here for transparency.merge_chromrequires ≥2 LD blocks per chromosome — a single-block chromosome fails atplink2 --pmerge-list("requires at least two filesets").