Skip to content

[FIX] concordant z is the statistic, not the inverse of its corrected p - #141

Merged
jdkent merged 2 commits into
neurostuff:masterfrom
jdkent:fix/concordant-z-tail
Aug 20, 2026
Merged

[FIX] concordant z is the statistic, not the inverse of its corrected p#141
jdkent merged 2 commits into
neurostuff:masterfrom
jdkent:fix/concordant-z-tail

Conversation

@jdkent

@jdkent jdkent commented Aug 20, 2026

Copy link
Copy Markdown
Member

The "concordant" mode reported norm.isf(p) of a p-value that had already been doubled to correct for testing both tails. That is a tail mismatch rather than a convention: it shrank the statistic to absorb a multiplicity penalty that belongs to the p-value alone.

Winkler et al. (2016), which the mode docstrings cite, define the concordant statistic as T = max(-2 sum ln p_k, -2 sum ln (1 - p_k)) -- the better of the two directed combinations, unshrunk -- and apply the two-test correction to its p-value. That is norm.isf(p / 2), and it now reproduces the winning directed statistic exactly (4e-16 over 1650 cases across both estimators).

Three symptoms had one cause. The reported z was negative wherever p > 0.5, carried the opposite sign to the effect there, and was -inf wherever the cap put p at exactly 1 -- so the least significant results had the largest magnitudes. A single input now combines to its own z; it was 2.7822 for an input of 3.0.

Thresholding the old map at any z_c selected p < sf(z_c) instead of p < 2 sf(z_c), so every concordant z-map ran at exactly half its intended two-sided alpha. The fix only raises |z|, so at a fixed threshold it can add voxels but never remove them.

p and logp are unchanged. The four pinned concordant values were not independent references: before 091b633 the tests computed ss.norm.isf(p) themselves and asserted it against a constant, and that commit moved the expression into the estimator and carried the constants across. They assert that norm.isf(p) equals norm.isf(p), and so could not detect this.

This completes #125, which gave the concordant statistic its direction but left the magnitude alone.

Adds a parametrized invariant that z and p describe the same tail in all three modes -- the check that would have caught it, and that the two right-tailed modes have always satisfied.

The "concordant" mode reported `norm.isf(p)` of a p-value that had already
been doubled to correct for testing both tails. That is a tail mismatch
rather than a convention: it shrank the statistic to absorb a multiplicity
penalty that belongs to the p-value alone.

Winkler et al. (2016), which the mode docstrings cite, define the concordant
statistic as T = max(-2 sum ln p_k, -2 sum ln (1 - p_k)) -- the better of the
two directed combinations, unshrunk -- and apply the two-test correction to
its p-value. That is `norm.isf(p / 2)`, and it now reproduces the winning
directed statistic exactly (4e-16 over 1650 cases across both estimators).

Three symptoms had one cause. The reported z was negative wherever p > 0.5,
carried the opposite sign to the effect there, and was -inf wherever the cap
put p at exactly 1 -- so the least significant results had the largest
magnitudes. A single input now combines to its own z; it was 2.7822 for an
input of 3.0.

Thresholding the old map at any z_c selected p < sf(z_c) instead of
p < 2 sf(z_c), so every concordant z-map ran at exactly half its intended
two-sided alpha. The fix only raises |z|, so at a fixed threshold it can add
voxels but never remove them.

`p` and `logp` are unchanged. The four pinned concordant values were not
independent references: before 091b633 the tests computed `ss.norm.isf(p)`
themselves and asserted it against a constant, and that commit moved the
expression into the estimator and carried the constants across. They assert
that norm.isf(p) equals norm.isf(p), and so could not detect this.

This completes neurostuff#125, which gave the concordant statistic its direction but
left the magnitude alone.

Adds a parametrized invariant that z and p describe the same tail in all
three modes -- the check that would have caught it, and that the two
right-tailed modes have always satisfied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jdkent jdkent left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some extra text

Comment thread pymare/estimators/combination.py Outdated
Comment thread pymare/results.py Outdated
Co-authored-by: James Kent <jamesdkent21@gmail.com>
@jdkent
jdkent merged commit d882dfb into neurostuff:master Aug 20, 2026
16 checks passed
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.11%. Comparing base (c9b3637) to head (d0ce4a3).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #141   +/-   ##
=======================================
  Coverage   95.11%   95.11%           
=======================================
  Files          13       13           
  Lines        1985     1985           
=======================================
  Hits         1888     1888           
  Misses         97       97           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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