fix(index): collapse per-node syncmer change record to one entry per position - #119
Merged
Conversation
amkram
force-pushed
the
fix/index-multitouch-dedup
branch
from
July 13, 2026 23:38
2c63825 to
6a31daa
Compare
…position
A node can touch the same syncmer position more than once -- e.g. a nuc mutation
SUBs/ADDs a syncmer, then that syncmer's block is dropped, DEL'ing it. The change
record then carries a stale non-DEL entry whose position a later DEL already erased
from the map. Three consumers assume one entry per position:
- index_single_mode::computeNewKminmerRanges: find() == end() -> "syncmer position
not found" abort (the placement/.idx build).
- mgsr::computeNewKminmerRanges: the found position's refOnSyncmers optional is empty
-> std::bad_optional_access (the --meta/.midx build); mgsr's block-deletion loop
also dereferences an already-deleted position's optional.
- the backtrack restores the wrong pre-node value.
Collapse each node's change record to one NET entry per position (net type from final
map membership, restore rsyncmer from the first touch) before it is consumed, in both
the sequential and parallel index_single_mode builders and in mgsr; also guard mgsr's
block-deletion loop against already-deleted positions.
No-op for panmans where no position repeats (rsv/sars/tb/ecoli/HIV/mtDNA): all unit
tests pass and sars's .midx is byte-identical before/after. Fixes indexing of
klebs_1000 (both .idx at k=51 and --meta), which deterministically hit these crashes.
amkram
force-pushed
the
fix/index-multitouch-dedup
branch
from
July 14, 2026 01:54
6a31daa to
38b3703
Compare
Owner
Author
|
@AlanZhangUCSC Could you review the changes to mgsr.cpp to see if this looks like a reasonable fix? Both modes currently broken on Klebsiella due to cases of a SNP followed by deleting that SNP's block. |
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.
Fixes
klebs_1000.panmancrash:.idx(--stop index):error: syncmer position not found in refOnSyncmersMap when computing new k-min-mer ranges(fails identically at k=51 and k=19)..midx(--meta):error: bad optional access.When a node both edits a seed and deletes that seed's block, we recorded the same position twice in its per-node change record leading to crash.
Fix: collapse each node's change record to one net entry per position.