Skip to content

Commit 2c2604b

Browse files
Julienclaude
authored andcommitted
Fix D6: match Clojure two-proportion test formula (+1 pseudocount)
Replace standard two-proportion z-test with Clojure's version that adds +1 pseudocount to all four inputs (stats.clj:18-33). This Laplace smoothing regularizes z-scores for small group sizes common in Polis. Signature change: two_prop_test(p1, n1, p2, n2) taking proportions → two_prop_test(succ_in, succ_out, pop_in, pop_out) taking raw counts. Updated both scalar and vectorized versions plus all callers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0324c31 commit 2c2604b

6 files changed

Lines changed: 224 additions & 101 deletions

File tree

delphi/docs/CLJ-PARITY-FIXES-JOURNAL.md

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,58 @@ significance filtering. Using different priors is intentional.
440440

441441
### What's Next
442442

443-
1. **PR 5 — Fix D6 (Two-proportion test)**: Add +1 pseudocount to all 4 inputs, change signature
444-
from proportions to raw counts.
443+
1. **PR 6 — Fix D7 (Repness metric)**: Change formula from `pa * (|pat| + |rat|)` to
444+
`ra * rat * pa * pat` (Clojure product formula).
445+
446+
---
447+
448+
## PR 5: Fix D6 — Two-Proportion Test Pseudocounts
449+
450+
### TDD steps
451+
1. **Baseline**: 1 failed (pakistan-incremental D2, pre-existing), 102 passed, 5 skipped, 143 xfailed, 2 xpassed
452+
2. **Red**: Rewrote `TestD6TwoPropTest` with new signature `two_prop_test(succ_in, succ_out, pop_in, pop_out)`
453+
and correct Clojure formula → 3 failures (TypeError: old function expects proportions)
454+
3. **Fix**: Replaced both `two_prop_test` and `two_prop_test_vectorized` with Clojure formula:
455+
add +1 to all 4 inputs (stats.clj:20), compute `pi1=(s+1)/(p+1)`, standard pooled z-test
456+
4. **Green**: All 3 D6 formula tests pass, 4 blob comparison tests xfail (depend on D10)
457+
5. **Full suite**: 4 regression failures, all in `rat`/`rdt`/`agree_metric`/`disagree_metric` — direct
458+
downstream of the formula change. No unexpected fields affected.
459+
6. **Re-recorded golden snapshots** for all 7 datasets (public + private)
460+
7. **Final**: 1 failed (pakistan-incremental D2, pre-existing), 102 passed, 5 skipped, 143 xfailed, 2 xpassed
461+
462+
### Changes
463+
- `repness.py`: `two_prop_test(p1, n1, p2, n2)``two_prop_test(succ_in, succ_out, pop_in, pop_out)`
464+
with +1 pseudocount on all 4 inputs, matching Clojure's `(map inc ...)` (stats.clj:20)
465+
- `repness.py`: `two_prop_test_vectorized` — same signature change
466+
- `repness.py`: Updated callers in `add_comparative_stats` and `compute_group_comment_stats_df`
467+
to pass raw counts `(na, other_na, ns, other_ns)` instead of `(pa, ns, other_pa, other_ns)`
468+
- `test_discrepancy_fixes.py`: Rewrote `TestD6TwoPropTest` with correct formula, 7 test cases,
469+
edge cases, and regularization effect test
470+
- `test_repness_unit.py`: Updated `test_two_prop_test`, `test_two_prop_test_vectorized`,
471+
`test_two_prop_test_vectorized_edge_cases` for new signature
472+
- `test_old_format_repness.py`: Updated `test_two_prop_test` for new signature
473+
474+
### Key insight: existing test had wrong expected formula
475+
The pre-existing D6 test computed expected values using `(succ+1)/(n+2)` — as if two pseudocounts
476+
were added to the denominator. But Clojure's `(map inc ...)` adds +1 to each value independently,
477+
giving `(succ+1)/(pop+1)`. The formula is a standard pooled z-test on the pseudocount-adjusted values,
478+
not a Beta distribution posterior.
479+
480+
### Session 8 (2026-03-13)
481+
482+
- Created branch `jc/clj-parity-d6-two-prop-test` on top of `jc/clj-parity-d5-prop-test`
483+
- Read Clojure source (stats.clj:18-33, repness.clj:97-100) to verify formula and call sites
484+
- Discovered the existing D6 test had wrong expected formula — fixed
485+
- TDD cycle: red (3 TypeError failures) → fix → green (3 pass, 4 xfail)
486+
- Updated all callers: both scalar (`add_comparative_stats`) and vectorized
487+
(`compute_group_comment_stats_df`) now pass raw counts
488+
- Full suite: 4 regression failures, all in rat/rdt/metric fields (expected)
489+
- Re-recorded golden snapshots for all 7 datasets
490+
- Final validation: 102 passed, 1 pre-existing failure (pakistan-incremental D2)
491+
492+
### What's Next
493+
494+
1. **PR 6 — Fix D7 (Repness metric)**: Change from `pa * (|pat| + |rat|)` to `ra * rat * pa * pat`.
445495

446496
---
447497

delphi/docs/PLAN_DISCREPANCY_FIXES.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ This plan's "PR N" labels map to actual GitHub PRs as follows:
2121
| (perf) | #2436 | Stack 10/10 | Speed up regression tests |
2222
| PR 3 (D9) | #2446 || Fix D9: z-score thresholds (one-tailed) |
2323
| PR 4 (D5) | #2448 | Stack 14/25 | Fix D5: proportion test formula |
24+
| PR 5 (D6) ||| Fix D6: two-proportion test pseudocounts |
2425

2526
Future fix PRs will be appended to the stack as they're created.
2627

@@ -467,7 +468,7 @@ By this point, we should have good test coverage from all the per-discrepancy te
467468
| D3 | K-smoother buffer | PR 10 || Fix |
468469
| D4 | Pseudocount formula | **PR 2** | **#2435** | **DONE**|
469470
| D5 | Proportion test | **PR 4** || **DONE**|
470-
| D6 | Two-proportion test | PR 5 || Fix |
471+
| D6 | Two-proportion test | **PR 5** || **DONE** |
471472
| D7 | Repness metric | PR 6 || Fix (with flag for old formula) |
472473
| D8 | Finalize cmt stats | PR 7 || Fix |
473474
| D9 | Z-score thresholds | **PR 3** | **#2446** | **DONE**|

delphi/polismath/pca_kmeans_rep/repness.py

Lines changed: 75 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -95,33 +95,51 @@ def prop_test(succ: int, n: int) -> float:
9595
return 2 * math.sqrt(n_pc) * (succ_pc / n_pc - 0.5)
9696

9797

98-
def two_prop_test(p1: float, n1: int, p2: float, n2: int) -> float:
98+
def two_prop_test(succ_in: int, succ_out: int, pop_in: int, pop_out: int) -> float:
9999
"""
100-
Two-proportion z-test.
101-
100+
Two-proportion z-test with +1 pseudocount on all inputs.
101+
102+
Matches Clojure's stats/two-prop-test (stats.clj:18-33):
103+
(let [[succ-in succ-out pop-in pop-out] (map inc [succ-in succ-out pop-in pop-out])
104+
pi1 (/ succ-in pop-in)
105+
pi2 (/ succ-out pop-out)
106+
pi-hat (/ (+ succ-in succ-out) (+ pop-in pop-out))]
107+
...)
108+
109+
The +1 pseudocount (Laplace smoothing) regularizes the z-score for small
110+
samples, preventing extreme values when group sizes are tiny.
111+
102112
Args:
103-
p1: First proportion
104-
n1: Number of observations for first proportion
105-
p2: Second proportion
106-
n2: Number of observations for second proportion
107-
113+
succ_in: Number of successes in the group (e.g., agrees)
114+
succ_out: Number of successes outside the group
115+
pop_in: Total votes in the group
116+
pop_out: Total votes outside the group
117+
108118
Returns:
109-
Z-score
119+
Z-score (positive means group proportion > other proportion)
110120
"""
111-
if n1 == 0 or n2 == 0:
121+
if pop_in == 0 or pop_out == 0:
112122
return 0.0
113-
114-
# Pooled probability
115-
p = (p1 * n1 + p2 * n2) / (n1 + n2)
116-
117-
# Standard error
118-
se = math.sqrt(p * (1 - p) * (1/n1 + 1/n2))
119-
120-
# Z-score calculation
123+
124+
# Add +1 pseudocount to all four inputs (Clojure: map inc)
125+
s1 = succ_in + 1
126+
s2 = succ_out + 1
127+
p1 = pop_in + 1
128+
p2 = pop_out + 1
129+
130+
pi1 = s1 / p1
131+
pi2 = s2 / p2
132+
pi_hat = (s1 + s2) / (p1 + p2)
133+
134+
if pi_hat == 1.0:
135+
# Clojure note (stats.clj:26-27): "this isn't quite right... could
136+
# actually solve this using limits" — returning 0 for now, matching Clojure.
137+
return 0.0
138+
139+
se = math.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2))
121140
if se == 0:
122141
return 0.0
123-
else:
124-
return (p1 - p2) / se
142+
return (pi1 - pi2) / se
125143

126144

127145
def comment_stats(votes: np.ndarray, group_members: List[int]) -> Dict[str, Any]:
@@ -182,15 +200,17 @@ def add_comparative_stats(comment_stats: Dict[str, Any],
182200
result['ra'] = result['pa'] / other_stats['pa'] if other_stats['pa'] > 0 else 1.0
183201
result['rd'] = result['pd'] / other_stats['pd'] if other_stats['pd'] > 0 else 1.0
184202

185-
# Calculate representativeness tests
203+
# Calculate representativeness tests — pass raw counts, matching Clojure's
204+
# (stats/two-prop-test (:na in-stats) (sum :na rest-stats)
205+
# (:ns in-stats) (sum :ns rest-stats)) (repness.clj:97-100)
186206
result['rat'] = two_prop_test(
187-
result['pa'], result['ns'],
188-
other_stats['pa'], other_stats['ns']
207+
result['na'], other_stats['na'],
208+
result['ns'], other_stats['ns']
189209
)
190-
210+
191211
result['rdt'] = two_prop_test(
192-
result['pd'], result['ns'],
193-
other_stats['pd'], other_stats['ns']
212+
result['nd'], other_stats['nd'],
213+
result['ns'], other_stats['ns']
194214
)
195215

196216
return result
@@ -493,30 +513,38 @@ def prop_test_vectorized(succ: pd.Series, n: pd.Series) -> pd.Series:
493513
return z
494514

495515

496-
def two_prop_test_vectorized(p1: pd.Series, n1: pd.Series,
497-
p2: pd.Series, n2: pd.Series) -> pd.Series:
516+
def two_prop_test_vectorized(succ_in: pd.Series, succ_out: pd.Series,
517+
pop_in: pd.Series, pop_out: pd.Series) -> pd.Series:
498518
"""
499-
Vectorized two-proportion z-test.
519+
Vectorized two-proportion z-test with +1 pseudocount on all inputs.
520+
521+
Matches Clojure's stats/two-prop-test (stats.clj:18-33).
522+
See two_prop_test() scalar version for formula details.
500523
501524
Args:
502-
p1: Series of first proportions
503-
n1: Series of number of observations for first proportion
504-
p2: Series of second proportions
505-
n2: Series of number of observations for second proportion
525+
succ_in: Series of success counts in the group
526+
succ_out: Series of success counts outside the group
527+
pop_in: Series of total vote counts in the group
528+
pop_out: Series of total vote counts outside the group
506529
507530
Returns:
508531
Series of z-scores
509532
"""
510-
# Pooled probability
511-
p_pooled = (p1 * n1 + p2 * n2) / (n1 + n2)
533+
# Add +1 pseudocount to all four inputs (Clojure: map inc)
534+
s1 = succ_in + 1
535+
s2 = succ_out + 1
536+
p1 = pop_in + 1
537+
p2 = pop_out + 1
512538

513-
# Standard error
514-
se = np.sqrt(p_pooled * (1 - p_pooled) * (1/n1 + 1/n2))
539+
pi1 = s1 / p1
540+
pi2 = s2 / p2
541+
pi_hat = (s1 + s2) / (p1 + p2)
515542

516-
# Z-score calculation
517-
z = (p1 - p2) / se
543+
se = np.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2))
544+
z = (pi1 - pi2) / se
518545

519-
# Handle edge cases
546+
# Handle edge cases: pop_in=0 or pop_out=0 → 0, pi_hat=1 → 0
547+
z = z.where((pop_in > 0) & (pop_out > 0), 0.0)
520548
z = z.fillna(0.0)
521549
z = z.replace([np.inf, -np.inf], 0.0)
522550
return z
@@ -649,14 +677,16 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame,
649677
stats_df['ra'] = stats_df['ra'].replace([np.inf, -np.inf], 1.0).fillna(1.0)
650678
stats_df['rd'] = stats_df['rd'].replace([np.inf, -np.inf], 1.0).fillna(1.0)
651679

652-
# Compute representativeness tests (two-proportion z-test: group vs other)
680+
# Compute representativeness tests — pass raw counts, matching Clojure's
681+
# (stats/two-prop-test (:na in-stats) (sum :na rest-stats)
682+
# (:ns in-stats) (sum :ns rest-stats)) (repness.clj:97-100)
653683
stats_df['rat'] = two_prop_test_vectorized(
654-
stats_df['pa'], stats_df['ns'],
655-
stats_df['other_pa'], stats_df['other_votes']
684+
stats_df['na'], stats_df['other_agree'],
685+
stats_df['ns'], stats_df['other_votes']
656686
)
657687
stats_df['rdt'] = two_prop_test_vectorized(
658-
stats_df['pd'], stats_df['ns'],
659-
stats_df['other_pd'], stats_df['other_votes']
688+
stats_df['nd'], stats_df['other_disagree'],
689+
stats_df['ns'], stats_df['other_votes']
660690
)
661691

662692
# Compute metrics

delphi/tests/test_discrepancy_fixes.py

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -798,30 +798,62 @@ def test_pat_values_match_clojure_blob(self, conv, clojure_blob, dataset_name):
798798
class TestD6TwoPropTest:
799799
"""
800800
D6: Python uses standard two-proportion z-test without pseudocounts.
801-
Clojure adds +1 pseudocount to all 4 inputs (succ1, n1, succ2, n2).
801+
Clojure adds +1 pseudocount to all 4 inputs (stats.clj:18-33):
802+
(map inc [succ-in succ-out pop-in pop-out])
803+
pi1 = (succ-in+1)/(pop-in+1), pi2 = (succ-out+1)/(pop-out+1)
804+
pi-hat = (succ-in+1 + succ-out+1) / (pop-in+1 + pop-out+1)
802805
"""
803806

804-
def test_two_prop_test_with_pseudocounts(self):
805-
"""two_prop_test should add +1 pseudocounts matching Clojure."""
806-
# With pseudocounts: (succ+1)/(n+2) for both groups
807-
succ1, n1 = 10, 20
808-
succ2, n2 = 15, 30
809-
810-
# Clojure formula adds +1 to successes and +2 to trials
811-
p1_clj = (succ1 + 1) / (n1 + 2)
812-
p2_clj = (succ2 + 1) / (n2 + 2)
813-
p_pooled_clj = (succ1 + succ2 + 2) / (n1 + n2 + 4)
814-
se_clj = math.sqrt(p_pooled_clj * (1 - p_pooled_clj) * (1 / (n1 + 2) + 1 / (n2 + 2)))
815-
expected = (p1_clj - p2_clj) / se_clj if se_clj > 0 else 0.0
816-
817-
# Python currently doesn't add pseudocounts
818-
p1_py = succ1 / n1
819-
p2_py = succ2 / n2
820-
python_result = two_prop_test(p1_py, n1, p2_py, n2)
821-
822-
print(f"two_prop_test: Python={python_result:.4f}, Clojure(with pseudocounts)={expected:.4f}")
823-
check.almost_equal(python_result, expected, abs=0.01,
824-
msg=f"two_prop_test should include pseudocounts: Python={python_result:.4f}, expected={expected:.4f}")
807+
@staticmethod
808+
def _clojure_two_prop_test(succ_in, succ_out, pop_in, pop_out):
809+
"""Reference implementation of Clojure's two-prop-test (stats.clj:18-33)."""
810+
s1, s2, p1, p2 = succ_in + 1, succ_out + 1, pop_in + 1, pop_out + 1
811+
pi1 = s1 / p1
812+
pi2 = s2 / p2
813+
pi_hat = (s1 + s2) / (p1 + p2)
814+
if pi_hat == 1:
815+
return 0.0
816+
return (pi1 - pi2) / math.sqrt(pi_hat * (1 - pi_hat) * (1/p1 + 1/p2))
817+
818+
def test_two_prop_test_matches_clojure_formula(self):
819+
"""two_prop_test(succ_in, succ_out, pop_in, pop_out) should match Clojure."""
820+
# Test cases: (succ_in, succ_out, pop_in, pop_out)
821+
test_cases = [
822+
(10, 15, 20, 30), # typical case
823+
(0, 0, 10, 10), # no successes in either group
824+
(5, 5, 10, 10), # identical groups
825+
(10, 0, 10, 10), # all success in group, none outside
826+
(1, 1, 1, 1), # minimal counts
827+
(50, 20, 100, 200), # asymmetric sizes
828+
(0, 10, 20, 30), # no success in group, some outside
829+
]
830+
831+
for succ_in, succ_out, pop_in, pop_out in test_cases:
832+
expected = self._clojure_two_prop_test(succ_in, succ_out, pop_in, pop_out)
833+
result = two_prop_test(succ_in, succ_out, pop_in, pop_out)
834+
check.almost_equal(
835+
result, expected, abs=0.001,
836+
msg=f"two_prop_test({succ_in},{succ_out},{pop_in},{pop_out}): "
837+
f"got={result:.4f}, expected={expected:.4f}")
838+
839+
def test_two_prop_test_edge_cases(self):
840+
"""Edge cases: n=0 should return 0, pi_hat=1 should return 0."""
841+
# n=0 cases
842+
check.equal(two_prop_test(5, 5, 0, 10), 0.0)
843+
check.equal(two_prop_test(5, 5, 10, 0), 0.0)
844+
# Both zero
845+
check.equal(two_prop_test(0, 0, 0, 0), 0.0)
846+
847+
def test_two_prop_test_pseudocount_effect(self):
848+
"""Pseudocounts should shrink z-scores toward zero for small samples."""
849+
# With small n, the +1 pseudocount has a large effect
850+
# succ=1, pop=1 → without pseudocount: p=1.0 (extreme)
851+
# With pseudocount: (1+1)/(1+1) = 1.0, but denominator also shifts
852+
result_small = two_prop_test(1, 0, 2, 2)
853+
result_large = two_prop_test(100, 0, 200, 200)
854+
# The large-sample z should be more extreme (less regularized)
855+
check.greater(abs(result_large), abs(result_small),
856+
"Large samples should produce more extreme z-scores than small ones")
825857

826858
@pytest.mark.xfail(reason="D6/D10: two-prop test differs + no shared comments to compare")
827859
def test_rat_values_match_clojure_blob(self, conv, clojure_blob, dataset_name):

delphi/tests/test_old_format_repness.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,17 @@ def test_prop_test(self):
5656
assert prop_test(0, 0) == 0.0
5757

5858
def test_two_prop_test(self):
59-
"""Test two-proportion z-test."""
60-
# Test cases
61-
assert np.isclose(two_prop_test(0.7, 100, 0.5, 100), 2.9, atol=0.1)
62-
assert np.isclose(two_prop_test(0.2, 50, 0.3, 50), -1.2, atol=0.1)
63-
64-
# Edge cases
65-
assert two_prop_test(0.5, 0, 0.5, 100) == 0.0
66-
assert two_prop_test(0.5, 100, 0.5, 0) == 0.0
59+
"""Test two-proportion z-test with +1 pseudocounts (Clojure parity)."""
60+
# two_prop_test(succ_in, succ_out, pop_in, pop_out) — raw counts
61+
# After +1: pi1=71/101≈0.703, pi2=51/101≈0.505, z≈2.88
62+
assert np.isclose(two_prop_test(70, 50, 100, 100), 2.88, atol=0.1)
63+
64+
# Equal proportions → z ≈ 0
65+
assert np.isclose(two_prop_test(25, 25, 50, 50), 0.0, atol=0.1)
66+
67+
# Edge cases: pop=0 → 0
68+
assert two_prop_test(5, 5, 0, 100) == 0.0
69+
assert two_prop_test(5, 5, 100, 0) == 0.0
6770

6871

6972
class TestCommentStats:

0 commit comments

Comments
 (0)