Skip to content

Fix off-by-one in HMM forward-backward posterior indexing#2519

Open
sirus20x6 wants to merge 1 commit intosamtools:developfrom
sirus20x6:fix/hmm-fwdbwd-offset
Open

Fix off-by-one in HMM forward-backward posterior indexing#2519
sirus20x6 wants to merge 1 commit intosamtools:developfrom
sirus20x6:fix/hmm-fwdbwd-offset

Conversation

@sirus20x6
Copy link
Copy Markdown
Contributor

Summary

  • Fix fwd[i*2 + state] to fwd[(i+1)*2 + state] in vcfroh.c and equivalent in vcfcnv.c — the fwd[0..nstates-1] slot contains initial state probabilities, not site 0 posteriors
  • Fix hmm_set_tprob() in HMM.c to reallocate tprob_arr when called with a larger ntprob, preventing heap buffer overflow

Test plan

  • Existing test suite passes (updated roh.1.{2,3,4}.out to match corrected posteriors)
  • Verify ROH quality scores are more accurate with corrected posteriors

The fwd-bwd probability array returned by hmm_get_fwd_bwd_prob()
reserves positions [0..nstates-1] for initial state probabilities,
so site i's posteriors start at (i+1)*nstates, not i*nstates. This
is encoded in the HMM_PPROB macro but was not followed in:
  - vcfroh.c flush_viterbi(): fwd[i*2+state] -> fwd[(i+1)*2+state]
  - vcfcnv.c cnv_flush_viterbi(): fwd+isite*nstates -> fwd+(isite+1)*nstates

Also fix hmm_set_tprob() in HMM.c: when called a second time with a
different ntprob, the old tprob_arr was never reallocated because the
guard was `if (!hmm->tprob_arr)`. Now compare against the previous
ntprob_arr value and realloc when the size changes.
@sirus20x6 sirus20x6 force-pushed the fix/hmm-fwdbwd-offset branch from 37ec6ad to 2de686a Compare March 26, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant