Repository navigation
Conversation
Add two CLI flags that gate Pass-2 sub-SMEM reseeding (the inner loop
in mb_seed_intv / mb_seed_intv_batch) on the SA-interval size of each
Pass-1 SMEM:
--max-sub-occ N (default 10) upper bound: skip Pass-2 for SMEMs with
occurrence > N (existing internal default,
now user-configurable; N=0 disables Pass-2
entirely)
--min-sub-occ N (default 1) lower bound: skip Pass-2 for SMEMs with
occurrence < N; intended as an ablation
for the "skip Pass-2 on uniquely-anchored
SMEMs" hypothesis
Default values preserve current behavior bit-for-bit (Pass-1 only ever
appends SMEMs with size > 0, so size < 1 never fires). Plumbed through
the mb_seed_intv / mb_seed_intv_batch signatures (mbpriv.h, seed.c) and
the three callers in map-algo.c (mb_map_seq + mb_map_batch) and
map-main.c (streaming worker_for_se_batch).
CLI option codes are 312 / 313 (310 and 311 are taken by --meth and
--hic on this branch). Documented in usage() output. Both flags gate on
the same SMEM SA-interval size field; symmetric naming
(max-/min-sub-occ) reflects that.
Smoke-tested on test/chrM-human.fa.gz: default behavior is bit-identical
whether the flags are omitted or set to their defaults explicitly.
Empirical measurement on WGS-1M R1-only against GRCh38 (1,000,000 SE
Illumina reads, hs38 ref, 8 threads), reproducing the numbers from the
prior implementation on the methylation branch:
Pass-2 ON Pass-2 OFF predA (size>=2)
(default) (--max=0) (--min=2)
Wall 14.6 s 6.4 s 12.4 s
CPU 54.5 s 33.0 s 39.6 s
shift60 vs ON - 34,675 reads 316 of 34,675 (0.91%)
shift60 = same-placement reads where Pass-2 ON deflates MAPQ from 60 to
<60. The lower-bound predicate preserves only 0.91% of those deflations
-- the dominant case is "uniquely-seeded SMEM with paralog signal hidden
inside it" -- so --min-sub-occ is a measurement knob, not a recommended
production setting.
Follow-up to issue #10 (Pass-2 sub-SMEM reseeding reduces MAPQ60
recall). Implementation ported from the methylation branch.
nh13
force-pushed
the
feat/submem-ablation
branch
from
August 7, 2026 20:52
8fb79a9 to
270cd04
Compare
Owner
Author
|
@coderabbitai pause |
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
Exposes the existing Pass-2 sub-SMEM reseeding threshold on the CLI, and adds a symmetric lower-bound knob for ablation studies. No change to default behavior.
What master already does
mb_seed_intv/mb_seed_intv_batchrun a two-pass seeding loop. Pass-1 collects standard SMEMs; Pass-2 reseeds inside each Pass-1 SMEM to find shorter sub-SMEMs that could anchor paralog hits. Pass-2 is gated on the Pass-1 SMEM's SA-interval size: a hard-codedopt->max_sub_occ = 10inoptions.cskips Pass-2 for any SMEM with size > 10. There was no lower bound, but Pass-1 only appends SMEMs withsize > 0, so the effective floor was 1.What this PR changes
--max-sub-occ N10N=0disables Pass-2 entirely (ablation knob).--min-sub-occ N1Defaults reproduce master bit-for-bit. Both flags gate on the same SMEM SA-interval size field; symmetric naming reflects that.
Plumbed through the
mb_seed_intv/mb_seed_intv_batchsignatures (mbpriv.h,seed.c) and the three callers inmap-algo.c(mb_map+mb_map_batch) andmap-main.c(streamingworker_for_se_batch). CLI option codes are 312 / 313 (310 and 311 are taken by--methand--hicon this branch).Follow-up to #10 (Pass-2 sub-SMEM reseeding reduces MAPQ60 recall).
Stacked on #11
This PR is stacked on top of #11 (
fix(map): drop is_pe param from mb_map_batch definition+ minimal build CI). Once #11 merges, GitHub will retarget this PR ontomasterautomatically. The base branch isfix/mb-map-batch-is-pe-build-breakso CI on this PR exercises the ablation feature on top of a buildable tree.Empirical measurement
WGS-1M R1-only against GRCh38 (1,000,000 SE Illumina reads, hs38 ref, 8 threads):
--max-sub-occ=0)--min-sub-occ=2)shift60= same-placement reads where Pass-2 ON deflates MAPQ from 60 to <60. The lower-bound predicate preserves only 0.91% of those deflations -- the dominant case is "uniquely-seeded SMEM with paralog signal hidden inside it" -- so--min-sub-occis a measurement knob, not a recommended production setting.Test plan
makeandmake -C api-testbuild cleanminibwa map --helplists--max-sub-occ=INT [10]and--min-sub-occ=INT [1]test/chrM-human.fa.gz, output is bit-identical betweenminibwa map -aandminibwa map -a --max-sub-occ=10 --min-sub-occ=1(defaults preserve master's behavior; chrM is too small to exercise Pass-2 deflation)