From b00638dce25867f70b87f6a57ac720145ea24270 Mon Sep 17 00:00:00 2001 From: Julien Cornebise Date: Mon, 27 Jul 2026 18:45:00 +0200 Subject: [PATCH] =?UTF-8?q?python-math=20#44:=20refactor(math):=20PR=2014b?= =?UTF-8?q?/14c=20=E2=80=94=20readable=20two-phase=20stats=20+=20blob-inje?= =?UTF-8?q?ction=20pins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What `GOAL_CUTOVER_READY.md` Phase 3, the clarity refactor (Julien ruling 2026-07-27: land the clean code PRE-cutover; spec in `HANDOFF_PR14_VECTORIZED_REFACTOR.md`). - 14c: `compute_group_comment_stats_df` is split into the plumbing (`_group_comment_vote_counts`: mapping, totals, cross-product, the `other_*` columns) and the statistics recipe (`_comment_stats_from_counts`: pseudocount probabilities, proportion tests on raw counts, representativeness ratios, two-proportion tests, signed metrics, the "repful" pick) — the recipe now reads like the scalar chain it replaced. Pure code motion: identical operations in identical order, with bit-identity guarded by the certification battery (which replays 20 recorded dataset entries through the engine and compares against Clojure reference recordings). - 14b: `TestBlobInjectionStats` injects the CLOJURE result blob's group memberships (unfolded through the blob's own base clusters) plus the dataset votes into the PRODUCTION stats path, and compares every repness entry in the blob per (gid, tid): n-success / n-trials / p-success / p-test / repness / repness-test / repful-for. Green on the `vw` AND `biodiversity` datasets (`repness-test` is compared at 2e-6 relative tolerance because Clojure emits it rounded). ## Also The three sub-threshold cleanups from the collapse-review agent: `CUTOVER_RUNBOOK.md` drops the stale engine-mode env line; the greedy-carry threshold test drops its now-duplicate `improved` parametrize label; two module docstrings updated to collapse-era wording. commit-id:acff8fbe --- delphi/docs/CUTOVER_RUNBOOK.md | 2 +- delphi/polismath/pca_kmeans_rep/repness.py | 41 +++++++++++- delphi/tests/test_in_conv_greedy_carry.py | 4 +- delphi/tests/test_mod_ptpt_leak_parity.py | 8 +-- delphi/tests/test_repness_unit.py | 76 +++++++++++++++++++++- 5 files changed, 122 insertions(+), 9 deletions(-) diff --git a/delphi/docs/CUTOVER_RUNBOOK.md b/delphi/docs/CUTOVER_RUNBOOK.md index cd78ab6a0..8f3ea4750 100644 --- a/delphi/docs/CUTOVER_RUNBOOK.md +++ b/delphi/docs/CUTOVER_RUNBOOK.md @@ -63,7 +63,7 @@ from Clojure's env, rows invisible to the server (UNIQUE(zid, math_env)). ``` docker compose --profile delphi-math up -d delphi-math-poller -# env: POLISMATH_ENGINE_MODE=clojure-legacy DELPHI_MATH_ENV=delphi +# env: DELPHI_MATH_ENV=delphi (engine has one path since the mode collapse) ``` Verify within minutes: diff --git a/delphi/polismath/pca_kmeans_rep/repness.py b/delphi/polismath/pca_kmeans_rep/repness.py index e228793d1..20cfa980a 100644 --- a/delphi/polismath/pca_kmeans_rep/repness.py +++ b/delphi/polismath/pca_kmeans_rep/repness.py @@ -183,7 +183,14 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, Vectorized port of Clojure's per-(group, comment) `comment-stats` recipe (math/src/polismath/math/repness.clj:64-100). Operates on all groups and - comments simultaneously. + comments simultaneously, in two phases: + + 1. :func:`_group_comment_vote_counts` — the DataFrame plumbing that + reduces (votes, group memberships) to one row of raw counts per + (group, comment): ``na``/``nd``/``ns`` for the group and + ``other_agree``/``other_disagree``/``other_votes`` for everyone else. + 2. :func:`_comment_stats_from_counts` — the statistics recipe, reading + like Clojure's scalar comment-stats/finalize-cmt-stats chain. Args: votes_long: Long-format DataFrame with columns: @@ -210,6 +217,25 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, - disagree_metric: metric for disagree representativeness - repful: 'agree' or 'disagree' based on which is more representative """ + counts_df = _group_comment_vote_counts(votes_long, group_clusters, tid_order) + if counts_df.empty: + return counts_df + return _comment_stats_from_counts(counts_df) + + +def _group_comment_vote_counts(votes_long: pd.DataFrame, + group_clusters: List[Dict[str, Any]], + tid_order: Optional[List[Any]] = None) -> pd.DataFrame: + """Phase 1 — the plumbing: reduce (votes, group memberships) to raw + per-(group, comment) counts. + + Returns a DataFrame indexed by (group_id, comment) with the group's + ``na``/``nd``/``ns``, the clustered-voter totals ``total_agree``/ + ``total_disagree``/``total_votes``, and the derived ``other_*`` columns + (everyone not in this group) — the exact inputs Clojure's comment-stats + recipe consumes. Empty result (correct schema) when there are no votes + or no clustered voters. + """ # Build participant -> group mapping ptpt_to_group = {} for group in group_clusters: @@ -304,6 +330,19 @@ def compute_group_comment_stats_df(votes_long: pd.DataFrame, stats_df['other_disagree'] = stats_df['total_disagree'] - stats_df['nd'] stats_df['other_votes'] = stats_df['total_votes'] - stats_df['ns'] + return stats_df + + +def _comment_stats_from_counts(stats_df: pd.DataFrame) -> pd.DataFrame: + """Phase 2 — the statistics recipe, on clean per-(group, comment) counts. + + Reads like Clojure's scalar chain (comment-stats -> add-comparative-stats + -> finalize-cmt-stats, repness.clj:64-100/:97-100/:178/:191-193): + probabilities with pseudocounts, proportion tests on raw counts, + representativeness ratios group-vs-other, two-proportion tests, the + signed metric products, and the repful side pick. Adds the stat columns + to ``stats_df`` (same frame, mutated in place) and returns it. + """ # Compute probabilities with pseudocounts (Bayesian smoothing) # For group stats_df['pa'] = (stats_df['na'] + PSEUDO_COUNT/2) / (stats_df['ns'] + PSEUDO_COUNT) diff --git a/delphi/tests/test_in_conv_greedy_carry.py b/delphi/tests/test_in_conv_greedy_carry.py index 1de370839..665e432d6 100644 --- a/delphi/tests/test_in_conv_greedy_carry.py +++ b/delphi/tests/test_in_conv_greedy_carry.py @@ -14,7 +14,7 @@ 2. Legacy mode admits the top (15 - n) voters when under 15, with ties broken by matrix row order (deterministic surrogate for Clojure's hash-order tie). 3. Legacy greedy admits PERSIST across ticks even once the conversation grows - past 15 threshold-qualifiers (the carry) — improved mode drops them. + past 15 threshold-qualifiers (the carry). 4. Threshold-qualifiers stay in across ticks in BOTH modes (monotonicity). """ @@ -114,7 +114,7 @@ def test_legacy_blob_in_conv_includes_greedy_admits(self, monkeypatch): class TestThresholdMonotonicity: - @pytest.mark.parametrize('mode', ['improved', 'clojure-legacy']) + @pytest.mark.parametrize('mode', ['clojure-legacy']) def test_qualifier_stays_in_across_ticks(self, monkeypatch, mode): _mode(monkeypatch, mode) conv = Conversation('mono').update_votes(_votes(_TICK1_SPECS)) diff --git a/delphi/tests/test_mod_ptpt_leak_parity.py b/delphi/tests/test_mod_ptpt_leak_parity.py index d157f213d..86f417216 100644 --- a/delphi/tests/test_mod_ptpt_leak_parity.py +++ b/delphi/tests/test_mod_ptpt_leak_parity.py @@ -8,10 +8,10 @@ repness. Python gained a real ban feature (mod_out_ptpts row drop in _apply_moderation, 2026-06-10) — correct, but a certification divergence. -Per Q1: 'clojure-legacy' mode must LEAK the ban exactly like Clojure (rows -kept everywhere); 'improved' mode keeps the fix. This supersedes the legacy- -mode premise of #2623's TestCarryPruneOnParticipantBan (banning can no longer -shrink the legacy clustering pool, so the carry-prune scenario cannot arise). +Per Q1 + the mode collapse (2026-07-27, bans dropped as a feature): the +engine LEAKS the ban exactly like Clojure (rows kept everywhere). This +supersedes #2623's TestCarryPruneOnParticipantBan (banning can no longer +shrink the clustering pool, so the carry-prune scenario cannot arise). """ import os diff --git a/delphi/tests/test_repness_unit.py b/delphi/tests/test_repness_unit.py index 6733ea19d..1de248b42 100644 --- a/delphi/tests/test_repness_unit.py +++ b/delphi/tests/test_repness_unit.py @@ -456,4 +456,78 @@ def test_other_votes_includes_other_group_pass(self): assert g1['other_votes'] == 2, ( f"group 1 other_votes should include group-0 PASS; " f"got {g1['other_votes']}" - ) \ No newline at end of file + ) + +class TestBlobInjectionStats: + """PR 14b: inject the CLOJURE blob's group memberships + the dataset's + votes into the PRODUCTION stats path (compute_group_comment_stats_df) + and compare per-(gid, tid) values against the blob's repness entries — + the non-tautological pin of the vectorized formulas against the oracle + (HANDOFF_PR14_VECTORIZED_REFACTOR.md task 2). + + The blob stores only the WINNING side's values; `repful-for` selects + which of our columns to compare (agree -> na/pa/pat/ra/rat, disagree -> + nd/pd/pdt/rd/rdt). `repness-test` is emitted ROUNDED (~7 significant + digits) by Clojure, hence its looser tolerance. + """ + + def _stats_and_blob(self, ds_name): + import json + from polismath.regression import get_dataset_files + from common_utils import create_test_conversation + + files = get_dataset_files(ds_name, blob_type='cold_start') + blob_path = files['math_blob'] + if not os.path.exists(blob_path): + pytest.skip(f"math blob for {ds_name} unavailable") + with open(blob_path) as fh: + blob = json.load(fh) + + conv = create_test_conversation(ds_name) + votes_long = conv.rating_mat.melt( + ignore_index=False, var_name='comment', value_name='vote' + ).reset_index(names='participant') + + # Blob group members are BASE-cluster ids; unfold through the + # blob's own base-clusters (NOT python's clustering — injection). + # create_test_conversation matrices carry STRING pids/tids; the blob + # carries ints — map on the way in (and look tids up as str below). + bc = blob['base-clusters'] + bid_to_pids = dict(zip(bc['id'], bc['members'])) + groups = [ + {'id': g['id'], + 'members': [str(pid) for bid in g['members'] + for pid in bid_to_pids[bid]]} + for g in blob['group-clusters'] + ] + stats = compute_group_comment_stats_df(votes_long, groups) + return stats, blob + + @pytest.mark.parametrize('ds_name', ['vw', 'biodiversity']) + def test_stats_match_blob_repness_entries(self, ds_name): + stats, blob = self._stats_and_blob(ds_name) + checked = 0 + for gid_str, entries in blob['repness'].items(): + gid = int(gid_str) + for e in entries: + tid = str(e['tid']) + if (gid, tid) not in stats.index: + pytest.fail(f"blob entry (gid={gid}, tid={tid}) missing " + f"from stats index") + row = stats.loc[(gid, tid)] + side = e['repful-for'] + n_col, p_col, pt_col, r_col, rt_col = ( + ('na', 'pa', 'pat', 'ra', 'rat') if side == 'agree' + else ('nd', 'pd', 'pdt', 'rd', 'rdt')) + assert row[n_col] == e['n-success'], (gid, tid, side) + assert row['ns'] == e['n-trials'], (gid, tid, side) + assert np.isclose(row[p_col], e['p-success'], + rtol=1e-9, atol=1e-12), (gid, tid, 'p-success') + assert np.isclose(row[pt_col], e['p-test'], + rtol=1e-9, atol=1e-12), (gid, tid, 'p-test') + assert np.isclose(row[r_col], e['repness'], + rtol=1e-9, atol=1e-12), (gid, tid, 'repness') + assert np.isclose(row[rt_col], e['repness-test'], + rtol=2e-6, atol=1e-9), (gid, tid, 'repness-test') + checked += 1 + assert checked >= 4, f"vacuous: only {checked} blob entries compared"