Skip to content

fix: symmetric N/* handling across strands in compute_pwm - #40

Merged
aviezerl merged 1 commit into
masterfrom
fix/pwm-n-strand-asymmetry
Jul 21, 2026
Merged

fix: symmetric N/* handling across strands in compute_pwm#40
aviezerl merged 1 commit into
masterfrom
fix/pwm-n-strand-asymmetry

Conversation

@aviezerl

Copy link
Copy Markdown
Contributor

Problem

Reported by a user via Tamar: how does compute_pwm treat N, and why does the forward branch use get_avg_log_prob() while the reverse branch uses log(0.25)?

Confirmed - it's a real strand asymmetry. In src/DnaPSSM.cpp, the two functions compute_pwm uses treat N/* differently on the two strands:

  • Forward: p->get_avg_log_prob() - the column's mean log-probability, (log pA + log pC + log pG + log pT)/4.
  • Reverse: c_log_quarter = log(0.25), a flat constant.

Since a normalized column sums to 1, these are equal only at a uniform column; at informative columns mean(log p) < log(0.25), so the forward strand penalizes an N more than the reverse strand.

Impact

For a bidirectional motif, a window and its reverse-complement must score identically. They do - until an N lands on an informative position:

no-N:   |seq - rc| = 0.0
with-N: seq = -8.24,  rc = -6.64,   |diff| = 1.60 nats

Only affects sequences containing N/* with bidirect = TRUE.

Fix

Use get_avg_log_prob() on both strands in integrate_energy and integrate_energy_max. That restores strand symmetry and matches the convention the symmetric likelihood routines (max_like_match, integrate_like_seg) already use on both strands - so avg is the codebase's intended convention and the reverse c_log_quarter was the outlier.

Out of scope (left as-is): integrate_like / update_like_vec carry a different asymmetry (forward log(0.25), reverse avg) but are learning/likelihood internals, not the compute_pwm energy path. like_thresh_match is intentionally symmetric (log(0.25) on both). The identical fix is being applied to misha's DnaPSSM.cpp (same shared code) in a companion PR.

Test

Added a strand-symmetry test: a motif-length window with an N on an informative column must score the same as its reverse-complement, for both max and logSumExp. Fails before, passes after. Full test suite green.

https://claude.ai/code/session_01PK3qefBGDoBwd9w5226FEq

compute_pwm() scored N (and the * wildcard) inconsistently between
strands: integrate_energy() and integrate_energy_max() used the column's
average log-probability (get_avg_log_prob) on the forward strand but a
flat log(0.25) (c_log_quarter) on the reverse strand. With bidirect=TRUE
this broke strand symmetry - a window and its reverse-complement scored
differently (~1.6 nats) whenever an N fell on an informative position.

Use get_avg_log_prob() on both strands, matching the forward branch and
the other likelihood routines (max_like_match, integrate_like_seg). Only
affects sequences containing N/*.

Claude-Session: https://claude.ai/code/session_01PK3qefBGDoBwd9w5226FEq
@aviezerl
aviezerl merged commit 6d68d46 into master Jul 21, 2026
6 checks passed
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