test: guard the p-value tail against 1 - cdf coming back - #36
Merged
Conversation
`2 * (1 - dist.cdf(abs(stat)))` returns exactly 0.0 once the statistic passes about 8.35, because cdf rounds to 1.0 well before the tail mass underflows. At |t| = 8.94 with df 4231 the true two-tailed value is 5.7e-19 and the expression gives 0. It reached a reader: crypto_perps_funding/02_labels rendered "HAC t(3 lags): -8.94 (p=0)". The sources were fixed on main by 9f7a72a, and this detector is what stops them regressing. It pins compute_ic_summary_stats, compute_ic_hac_stats, Ljung-Box and the DSR numerically, and scans the rest statically. The scan parses rather than greps, because the expression has a second spelling that a `1 - <dist>.cdf(` pattern does not see: probability = float(norm.cdf(z_score)) p_value = float(1 - probability) It collects the names each file binds to a CDF value and flags `1 - <that name>` wherever it appears, so both spellings are covered. Verified: 14 passed against main unmodified, so main is clean. Reintroducing the expression at metrics/ic_inference.py:57 fails three of them - the two numeric pins and the static scan - so the guard is not vacuous. Taken from origin/fix/pvalue-survival-function, whose 18 source fixes main has since landed by another route; that branch is now conflicting and this file is the only part of it main does not have. PR #28 should close unmerged in favour of this.
stefan-jansen
added
compatibility: none
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #32.
2 * (1 - dist.cdf(abs(stat)))returns exactly0.0once the statistic passes about 8.35, becausecdfrounds to1.0well before the tail mass underflows. At |t| = 8.94 with df 4231 the true two-tailed value is 5.7e-19 and the expression gives 0. It reached a reader:crypto_perps_funding/02_labelsrenderedHAC t(3 lags): -8.94 (p=0).The sources are already fixed on
main, by9f7a72a, not by PR #28.metrics/ic_inference.py:57,283usestats.t.sf, and neither spelling of the expression appears anywhere insrc/. What main does not have is the guard.What this adds
tests/test_evaluation/test_pvalue_tail_precision.py, taken unchanged fromfix/pvalue-survival-function. It pinscompute_ic_summary_stats,compute_ic_hac_stats, Ljung-Box and the DSR numerically, and scans the rest statically.The scan parses rather than greps, because the expression has a second spelling a
1 - <dist>.cdf(pattern cannot see:It collects the names each file binds to a CDF value and flags
1 - <that name>wherever it appears, so both spellings are covered.Verification
14 pass against
mainunmodified, so main is clean. Reintroducing the expression atmetrics/ic_inference.py:57fails three of them - the two numeric pins and the static scan - so the guard is not vacuous.On PR #28
That branch carries 18 source fixes main has since landed by another route, is
CONFLICTINGagainst main, and this file is the only part of it main lacks. It should close unmerged in favour of this.