From 5f2cf64416cd0803ebd93f5ae869ff28508c01c4 Mon Sep 17 00:00:00 2001 From: aurascoper Date: Mon, 27 Jul 2026 12:32:31 -0500 Subject: [PATCH] fix(eval): stop zero-variance Cohen's d from reporting no effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cohens_d returned 0.0 whenever pooled standard deviation was zero, regardless of whether the means were identical or completely separated. Two perfectly separated constant groups therefore reported "no effect" — the least correct available answer, and one that ranks alongside genuine null results. Zero pooled variance now distinguishes the two cases: equal constant groups 0.0 a true zero separated constant groups NaN undefined, not absent NaN rather than infinity: d is undefined without within-group variation, NaN does not pretend the effect is absent, it matches the function's existing under-sized-sample behaviour, and it forces callers to treat the estimate as degenerate instead of ranking it as an ordinary finite effect. Finishes a consolidation the module already claimed. eval_stats.py's docstring says it consolidates cohens_d "previously duplicated in embedding_analyze.py and statistical_analysis.py", but the duplicates were never removed — and analyze_results.py had a fourth copy. All three now import the shared implementation, following the established `sys.path.insert` + `from eval_stats import ...` idiom already used for bootstrap_ci. Fixing one copy would otherwise have left three wrong ones. test_cohens_d_large_effect conflated "large" with "zero-variance degenerate"; it is replaced by three tests that separate identical-constant, separated- constant, and large-non-degenerate cases. Removes the first of three deselections in the python-contracts job (issue #38). Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 7 +++---- Evaluation/scripts/analyze_results.py | 15 ++++----------- Evaluation/scripts/embedding_analyze.py | 13 +++---------- Evaluation/scripts/eval_stats.py | 12 +++++++++--- Evaluation/scripts/statistical_analysis.py | 14 +++----------- Tests/eval/test_eval_stats.py | 16 +++++++++++++--- 6 files changed, 35 insertions(+), 42 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d21a343..4a45a0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,13 +112,12 @@ jobs: # test_joint_embedding.py imports mlx.core (Apple Silicon only) and would # fail collection here — run it locally. # - # The three deselected cases fail on this base and are tracked separately; - # they are pre-existing (issue #38), reproduce on both NumPy 1.26 and 2.4, and - # are not caused by this workflow. The rest of their files still run. + # The remaining deselected cases are pre-existing failures tracked in + # issue #38, reproducing on both NumPy 1.26 and 2.4. They are removed one + # at a time as each is resolved; the rest of their files still run. - name: Test (eval contract suites) run: | pytest Tests/eval -v \ --ignore=Tests/eval/test_joint_embedding.py \ - --deselect Tests/eval/test_eval_stats.py::test_cohens_d_large_effect \ --deselect Tests/eval/test_eval_stats.py::test_mann_whitney_u_disjoint \ --deselect Tests/eval/test_embedding_space.py::test_cka_independent diff --git a/Evaluation/scripts/analyze_results.py b/Evaluation/scripts/analyze_results.py index 0f48991..0e1ad9c 100644 --- a/Evaluation/scripts/analyze_results.py +++ b/Evaluation/scripts/analyze_results.py @@ -14,9 +14,13 @@ import json import math import statistics +import sys from pathlib import Path import numpy as np + +sys.path.insert(0, str(Path(__file__).parent)) +from eval_stats import cohens_d import pandas as pd REPO_ROOT = Path(__file__).resolve().parent.parent.parent @@ -39,17 +43,6 @@ def bootstrap_ci(data, confidence=0.95, n_boot=10000): return (lo, hi) -def cohens_d(a, b): - """Cohen's d effect size (pooled SD).""" - a, b = np.array(a, dtype=float), np.array(b, dtype=float) - if len(a) < 2 or len(b) < 2: - return float("nan") - pooled_std = math.sqrt((a.var(ddof=1) + b.var(ddof=1)) / 2) - if pooled_std == 0: - return 0.0 - return float((a.mean() - b.mean()) / pooled_std) - - def safe_mean(values): return float(np.nanmean(values)) if len(values) > 0 else float("nan") diff --git a/Evaluation/scripts/embedding_analyze.py b/Evaluation/scripts/embedding_analyze.py index 786678f..1f0b521 100644 --- a/Evaluation/scripts/embedding_analyze.py +++ b/Evaluation/scripts/embedding_analyze.py @@ -27,6 +27,9 @@ import numpy as np from scipy import stats as sp_stats + +sys.path.insert(0, str(Path(__file__).parent)) +from eval_stats import cohens_d from sklearn.cluster import KMeans from sklearn.preprocessing import StandardScaler @@ -50,16 +53,6 @@ def bootstrap_ci(data, confidence=0.95, n_boot=10000): return (lo, hi, len(data)) -def cohens_d(a, b): - a, b = np.array(a, dtype=float), np.array(b, dtype=float) - if len(a) < 2 or len(b) < 2: - return float("nan") - pooled_std = math.sqrt((a.var(ddof=1) + b.var(ddof=1)) / 2) - if pooled_std == 0: - return 0.0 - return float((a.mean() - b.mean()) / pooled_std) - - def mann_whitney_u(a, b): a, b = np.array(a, dtype=float), np.array(b, dtype=float) if len(a) < 2 or len(b) < 2: diff --git a/Evaluation/scripts/eval_stats.py b/Evaluation/scripts/eval_stats.py index f7a0aaa..24c6650 100644 --- a/Evaluation/scripts/eval_stats.py +++ b/Evaluation/scripts/eval_stats.py @@ -32,14 +32,20 @@ def bootstrap_ci(data, confidence=0.95, n_boot=10000): def cohens_d(a, b): - """Cohen's d effect size between two samples.""" + """Cohen's d effect size between two samples. + + Returns NaN where d is undefined rather than a finite value that would rank + alongside real effects: under-sized samples, and zero pooled variance with + separated means. Only equal constant groups are a true zero. + """ a, b = np.array(a, dtype=float), np.array(b, dtype=float) if len(a) < 2 or len(b) < 2: return float("nan") + mean_difference = float(a.mean() - b.mean()) pooled_std = math.sqrt((a.var(ddof=1) + b.var(ddof=1)) / 2) if pooled_std == 0: - return 0.0 - return float((a.mean() - b.mean()) / pooled_std) + return 0.0 if mean_difference == 0 else float("nan") + return mean_difference / pooled_std def mann_whitney_u(a, b): diff --git a/Evaluation/scripts/statistical_analysis.py b/Evaluation/scripts/statistical_analysis.py index bfa3a15..eb18519 100644 --- a/Evaluation/scripts/statistical_analysis.py +++ b/Evaluation/scripts/statistical_analysis.py @@ -31,6 +31,9 @@ import numpy as np from scipy import stats as sp_stats + +sys.path.insert(0, str(Path(__file__).parent)) +from eval_stats import cohens_d from sklearn.cluster import KMeans from sklearn.preprocessing import StandardScaler @@ -55,17 +58,6 @@ def bootstrap_ci(data, confidence=0.95, n_boot=10000): return (lo, hi, len(data)) -def cohens_d(a, b): - """Cohen's d effect size with pooled SD.""" - a, b = np.array(a, dtype=float), np.array(b, dtype=float) - if len(a) < 2 or len(b) < 2: - return float("nan") - pooled_std = math.sqrt((a.var(ddof=1) + b.var(ddof=1)) / 2) - if pooled_std == 0: - return 0.0 - return float((a.mean() - b.mean()) / pooled_std) - - def mann_whitney_u(a, b): """Mann-Whitney U test — non-parametric, no normality assumption.""" a, b = np.array(a, dtype=float), np.array(b, dtype=float) diff --git a/Tests/eval/test_eval_stats.py b/Tests/eval/test_eval_stats.py index 558ed96..ba00050 100644 --- a/Tests/eval/test_eval_stats.py +++ b/Tests/eval/test_eval_stats.py @@ -26,9 +26,19 @@ def test_cohens_d_identical(): assert d == 0.0 -def test_cohens_d_large_effect(): - d = cohens_d([1, 1, 1, 1], [5, 5, 5, 5]) - assert d > 2.0 # very large effect +def test_cohens_d_identical_constant_groups_are_zero(): + assert cohens_d([1, 1, 1, 1], [1, 1, 1, 1]) == 0.0 + + +def test_cohens_d_separated_constant_groups_are_undefined(): + # Zero pooled variance with separated means: d is unbounded, not absent. + # Returning 0.0 here would rank total separation as "no effect". + assert np.isnan(cohens_d([1, 1, 1, 1], [5, 5, 5, 5])) + + +def test_cohens_d_large_nondegenerate_effect(): + d = cohens_d([0.9, 1.0, 1.1, 1.0], [4.9, 5.0, 5.1, 5.0]) + assert abs(d) > 2.0 def test_mann_whitney_u_disjoint():