Compute tail p-values with sf, not 1 - cdf - #28
Open
stefan-jansen wants to merge 3 commits into
Open
Conversation
Every two-tailed p-value in the package was written as 2 * (1 - dist.cdf(abs(stat))). That expression returns exactly 0.0 once abs(stat) passes roughly 8.35 and is already wrong by 60% at 8.3, because cdf rounds to 1.0 long before the tail mass underflows and the subtraction cancels the remaining significant digits. dist.sf evaluates the tail directly. 15 sites: compute_ic_summary_stats and compute_ic_hac_stats, the conditional-IC test, the two binary-metric z-tests, the three event-study tests, the Ljung-Box test in factor validation, the regularized-factor alpha and beta p-values, the event-study plot annotation, and the tearsheet z-test. No decision changes: any statistic in the underflow zone is significant under any threshold. What changes is that a reader no longer sees a p-value of exactly zero, and stored p_value columns no longer carry 0.0. tests/test_evaluation/test_pvalue_tail_precision.py covers the two IC functions and Ljung-Box numerically, and guards the remaining sites with a static scan for the 1 - <dist>.cdf( pattern.
`probability = norm.cdf(z)` followed by `p_value = 1 - probability` cancels exactly as hard as `1 - norm.cdf(z)`: it reads as arithmetic rather than as a tail, which is why the first pass missed it and why the regex missed it too. Three more sites: the DSR p-value in both deflated_sharpe_ratio entry points, and the PSR p-value in the trade dashboard's statistical validation tab, which now takes the left tail of the z-score the same call already returns. The scan collects names bound to a CDF value per file and flags `1 - <name>` wherever it appears, so both spellings are covered. It reads code only: a comment explaining why a nearby line uses sf is prose, not the pattern. test_deflated_sharpe_pvalue_survives_extreme_z pins the numeric case. A Sharpe of 0.5 over ten years puts z above 25, so `probability` is 1.0 to the last bit while the true p-value is 2.5e-139.
The line-based version only saw an assignment whose name and .cdf( call
were on the same physical line, so a normally formatted
probability = float(
stats.norm.cdf(z_score)
)
was not a CDF binding and the `1 - probability` after it went unflagged.
The regex also needed comments stripped by hand, and that hack was
load-bearing.
ast removes both. An assignment is one node however it is wrapped, and
comments and docstrings are not in the tree at all, so prose naming the
pattern is not the pattern.
Eight cases pin the detector itself: four layouts it must catch, four
kinds of correct code it must not. The one shape it still misses - a CDF
reached through a subscript rather than a bare name - is stated in the
docstring rather than left to be discovered.
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.
The defect
A tail probability written as
1 - dist.cdf(x)returns exactly0.0oncexpassesroughly 8.35, and is already wrong by 60% at 8.3:
cdfrounds to1.0long before thetail mass underflows, so the subtraction cancels every remaining significant digit.
dist.sfevaluates the tail directly.Measured on
stats.twithdf=4231:2 * (1 - cdf)2 * sfcompute_ic_hac_statsis the one that surfaced it: a downstream notebook renderedHAC t(3 lags): -8.94 (p=0)to a reader. The true value is 5.7e-19.It appears in two spellings. The inline one is grep-visible. The other is not:
That cancels exactly as hard and reads as arithmetic rather than as a tail, which is why
the first pass over this package missed it.
What changed
18 sites.
Inline form (14):
metrics/ic_inference.py-compute_ic_summary_stats,compute_ic_hac_statsmetrics/conditional.py- conditional-IC t-testevaluation/binary_metrics.py- the two proportion z-tests (one-sidedgreaterandtwo-sided branches)
evaluation/event_analysis.py- the three event-study testsevaluation/factor/validation.py- the Ljung-Box residual testevaluation/factor/regularized.py- alpha and beta p-valuesvisualization/signal/event_plots.py- the event-plot annotationvisualization/backtest/tearsheet.py- the tearsheet z-testTwo-line form (4):
evaluation/stats/deflated_sharpe_ratio.py- the DSR p-value in both entry points.probabilityis untouched; it is the DSR itself and is meant to saturate. Onlyp_valuechanges, tonorm.sf(z_score).evaluation/trade_dashboard/tabs/stat_validation.py- the PSR p-value, which now takesthe left tail of the z-score
probabilistic_sharpe_ratio(..., return_components=True)already returns.
evaluation/factor/validation.pyand the four two-line sites were not in the originalreport; they are the same defect and are fixed here.
No decision changes: any statistic in the underflow zone is significant under any
threshold, so no verdict, winner or selection moves. What changes is that a reader no
longer sees a p-value of exactly zero, and stored
p_valuecolumns no longer carry0.0.Test
tests/test_evaluation/test_pvalue_tail_precision.pycoverscompute_ic_summary_stats,compute_ic_hac_stats, Ljung-Box and the DSR numerically - each with a fixture whosestatistic lands past the cancellation point and short of where the true tail mass
underflows - and guards the rest with a static scan of
src/.The scan handles both spellings: it collects the names each file binds to a CDF value,
then flags
1 - <that name>wherever it appears. It reads code only, so a commentexplaining why a nearby line uses
sfis prose, not the pattern.Against
main's source the numeric tests and the scan fail. On this branch all 6 pass,and
tests/test_evaluation tests/metrics tests/test_visualization tests/test_binary_metrics.pyis 3256 passed, 4 skipped.