Repository navigation
Conversation
Add p_value to IIDBootstrap that inverts the confidence interval built from the same bootstrap replications, so the p-value matches conf_int for every interval method. Supports two-sided, less, and greater alternatives and scalar or per-parameter null values. Signed-off-by: LuYuan-Zjyy <181187014+LuYuan-Zjyy@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1023 +/- ##
==========================================
- Coverage 99.51% 99.49% -0.03%
==========================================
Files 152 152
Lines 34209 34374 +165
Branches 2588 2610 +22
==========================================
+ Hits 34043 34200 +157
- Misses 94 98 +4
- Partials 72 76 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Sounds like a good idea. |
bashtage
left a comment
There was a problem hiding this comment.
Thanks for this. Inverting the interval is a good way to keep p_value consistent with conf_int. The refactor of conf_int is mechanical, and its existing tests pass.
I checked the inversion for every method. I called conf_int(..., reuse=True) after p_value, so both use the same replications. The interval at size = 1 - p then has the null on its boundary, to within 1e-17, for basic, percentile, norm, bc and bca. The two-sided percentile p-value agrees with 2 * min(F, 1 - F) from the bootstrap distribution to within 0.001.
Some things need to change:
-
A null that is NaN or
Nonegives p = 0.np.asarray(None, dtype=float)is NaN, and NaN is never inside an interval. Sop_value(func, null=np.nan)andp_value(func, null=None)both return[0., 0.], the strongest possible rejection, andnull=[0.0, np.nan]returns[0.40, 0.]._check_nullshould raise aTypeErrorforNoneand aValueErrorfor NaN, like the other argument checks. -
nullis checked only after the bootstrap has run. Anullwith the wrong length raises only afterfunchas been calledreps + 1times: 501 calls withreps=500. Withmethod="studentized"and nostd_err_func, the nested bootstrap runs first as well. The type, the dimension and NaN can be checked before any computation. The length needs the number of parameters, and evaluatingfunconce on the original data gives it before the bootstrap starts. -
Speed. For each parameter, the search calls
_conf_int_from_estimates60 times, and each call computes the interval for every parameter. The cost therefore grows with the square of the number of parameters. With BCa, each call also recomputes the jackknife acceleration, which does not depend onsize. These are times with 1,000 observations and 999 replications:Method, parameters conf_intp_valuebasic, 2 0.05 s 0.09 s basic, 50 0.08 s 14.2 s bca, 2 0.10 s 5.8 s bca, 10 0.12 s 36.6 s There are two ways to bring the cost close to that of
conf_int:- Compute the bias and the acceleration once.
- Search for each parameter without recomputing the others.
The intervals can also be inverted directly. Percentile, basic, norm and studentized invert through the bootstrap distribution. BC and BCa invert through the inverse of their adjusted percentile.
-
"Matches
conf_intby construction" holds only for the same replications.p_valuealways draws new replications and has noreuseargument, so aconf_intcall followed byp_valueuses different draws. With the null set to the lower bound of a 95% interval fromconf_int, the one-sided p-values were 0.034 and 0.026 instead of 0.025. The Notes of the docstring and the new section of the docs say thatp_valueuses the same replications asconf_int. There are two options:- Add
reuse, with the same conditions asconf_int. - Say that the p-value matches
conf_int(..., reuse=True)called afterp_value.
- Add
-
The docstring example fails as a doctest.
bs.p_value(...)returns an array, but the example shows no output. Also,ycomes from the globalnp.random.randn, which isn't seeded.pytest --doctest-modules arch/bootstrap/base.pyfails on this example, and the other examples in the file pass. Either seed aGeneratorand show the output, or assign the result as theconf_intexample does. -
Tests. Only the percentile method is compared with
conf_int. Most other tests only check that the p-values are between 0 and 1. The one-sided test puts the null 1.0 away from the estimate, so it only tells 0 apart from 1. Suggestions:- Parametrize the inversion test over every method and alternative. The interval at
size = 1 - pshould have the null on its boundary.conf_intcan't reuse the replications of the studentized method, so for that method compare with a fresh instance that has the same seed. - Add a block bootstrap,
IndependentSamplesBootstrapand a vectornull. - Test the errors: a
nullthat is NaN,None, a string or the wrong length, each checked before the bootstrap runs.
codecov/patch fails because no test covers the
TypeErrorbranch of_check_null. - Parametrize the inversion test over every method and alternative. The interval at
Smaller points:
tail_namecomes from a dict lookup, so it is astr. mypy reportsArgument 2 to "_conf_int_from_estimates" of "IIDBootstrap" has incompatible type "str"; expected "Literal['two', 'upper', 'lower']". Annotating the dict fixes this.studentized = "studentized"inp_valueis left over, since the extracted method now uses the literal.- The p-value is exactly 0 when every replication falls on one side of the null, for example with
null=10.0. The docstring should say so, and that the resolution is about1 / reps.
Validate null before the bootstrap runs, reuse cached replications, and search each parameter without rebuilding the others. Fix the studentized standard error and cover every interval method and alternative in the inversion tests. Signed-off-by: LuYuan-Zjyy <181187014+LuYuan-Zjyy@users.noreply.github.com>
Signed-off-by: LuYuan-Zjyy <181187014+LuYuan-Zjyy@users.noreply.github.com>
|
Thanks for the detailed review. I addressed the points:
Local: |
|
Thanks. |
| return null_arr | ||
|
|
||
|
|
||
| def _broadcast_null(name: str, null_arr: np.ndarray, num_params: int) -> Float64Array: |
What does this PR do?
Closes #800.
Adds
p_value()toIIDBootstrap(and, through inheritance, the block bootstraps) by inverting the confidence interval built from the same bootstrap replications. The p-value therefore matchesconf_int()by construction, for every interval method.alternative="two-sided"(default),"less", or"greater".nullmay be a scalar or one value per parameter.basic,percentile,studentized,norm,bc, andbcainterval methods.conf_int()into_conf_int_from_estimates(), so no per-method p-value formula is duplicated.Why this shape
The issue asks for the ability to obtain a p-value rather than a fixed-alpha interval. Inverting the existing interval keeps the p-value and the interval consistent automatically, and it remains valid as new interval methods are added.
Verification
pytest arch/tests/bootstrap- 2125 passed.ruff checkandruff format --checkon the changed files - passed.git diff --check- clean.std_err_func, BCa on a scalar statistic, and argument validation.