Three tests in Tests/eval fail on docs/eeg-methods-scope. They are
pre-existing, reproduce identically on NumPy 1.26.4 and 2.4.6, and surfaced only
because #37 puts these suites under CI for the first time. #37 deselects them by
name so the other 59 tests can gate; this issue tracks resolving them.
My reading is that two are wrong tests and one is a real defect, but all
three are statistical judgement calls, so none were changed unilaterally.
1. test_cohens_d_large_effect — likely a real defect
cohens_d([1, 1, 1, 1], [5, 5, 5, 5]) # returns 0.0, test asserts > 2.0
Evaluation/scripts/analyze_results.py:48:
if pooled_std == 0:
return 0.0
Both groups are constant, so pooled SD is 0 and Cohen's d is undefined
(unbounded). Returning 0.0 reports "no effect" for two perfectly separated
groups — the most misleading available sentinel. The function already returns
nan for the other degenerate case (len < 2), so nan would be consistent,
and inf would be defensible.
Duplicated in Evaluation/scripts/embedding_analyze.py:57, so any fix should
cover both.
The test input is degenerate, so the test could reasonably be rewritten too —
but a zero-variance guard that silently claims no effect is worth fixing on its
own merits, since real evaluation data can produce constant groups.
2. test_mann_whitney_u_disjoint — the test asks for the impossible
mann_whitney_u([1, 2, 3], [10, 11, 12]) # p = 0.1, test asserts < 0.05
With n₁ = n₂ = 3 the smallest attainable two-sided p is 2/20 = 0.1, which is
what the implementation returns for completely disjoint groups. No correct
implementation can pass this assertion at that sample size. The fix is in the
test: raise n to at least 4 per group, or assert p <= 0.1.
3. test_cka_independent — the expectation ignores estimator bias
X = randn(50, 128); Y = randn(50, 256)
cka(X, Y) # 0.781, test asserts < 0.3
Evaluation/scripts/embedding_space_analysis.py:37 implements standard
biased linear CKA (Kornblith et al. 2019) and centers correctly. The biased
estimator is known to be inflated when dimensionality exceeds sample count, and
here d ≫ n (128 and 256 versus 50), so ~0.78 on independent data is expected
behaviour rather than a bug.
Two ways out, and they answer different questions:
- keep the biased estimator and fix the test — raise n well above d, or relax
the threshold to reflect the bias at these shapes;
- switch to unbiased CKA, which is the better default when the tool is used to
compare representations of differing width, as the test itself does.
The second changes reported numbers in any existing analysis, so it is a
research decision, not a cleanup.
Reproduce
pip install pytest numpy pandas scipy
pytest Tests/eval/test_eval_stats.py Tests/eval/test_embedding_space.py -v
Closing condition
All three pass, and the --deselect lines are removed from the
python-contracts job in .github/workflows/ci.yml.
Three tests in
Tests/evalfail ondocs/eeg-methods-scope. They arepre-existing, reproduce identically on NumPy 1.26.4 and 2.4.6, and surfaced only
because #37 puts these suites under CI for the first time. #37 deselects them by
name so the other 59 tests can gate; this issue tracks resolving them.
My reading is that two are wrong tests and one is a real defect, but all
three are statistical judgement calls, so none were changed unilaterally.
1.
test_cohens_d_large_effect— likely a real defectEvaluation/scripts/analyze_results.py:48:Both groups are constant, so pooled SD is 0 and Cohen's d is undefined
(unbounded). Returning
0.0reports "no effect" for two perfectly separatedgroups — the most misleading available sentinel. The function already returns
nanfor the other degenerate case (len < 2), sonanwould be consistent,and
infwould be defensible.Duplicated in
Evaluation/scripts/embedding_analyze.py:57, so any fix shouldcover both.
The test input is degenerate, so the test could reasonably be rewritten too —
but a zero-variance guard that silently claims no effect is worth fixing on its
own merits, since real evaluation data can produce constant groups.
2.
test_mann_whitney_u_disjoint— the test asks for the impossibleWith n₁ = n₂ = 3 the smallest attainable two-sided p is 2/20 = 0.1, which is
what the implementation returns for completely disjoint groups. No correct
implementation can pass this assertion at that sample size. The fix is in the
test: raise n to at least 4 per group, or assert
p <= 0.1.3.
test_cka_independent— the expectation ignores estimator biasEvaluation/scripts/embedding_space_analysis.py:37implements standardbiased linear CKA (Kornblith et al. 2019) and centers correctly. The biased
estimator is known to be inflated when dimensionality exceeds sample count, and
here d ≫ n (128 and 256 versus 50), so ~0.78 on independent data is expected
behaviour rather than a bug.
Two ways out, and they answer different questions:
the threshold to reflect the bias at these shapes;
compare representations of differing width, as the test itself does.
The second changes reported numbers in any existing analysis, so it is a
research decision, not a cleanup.
Reproduce
Closing condition
All three pass, and the
--deselectlines are removed from thepython-contractsjob in.github/workflows/ci.yml.