feat(proportion.single.inequality): add exact test#334
Conversation
Documentation build overview
|
There was a problem hiding this comment.
Code Review
This pull request introduces exact binomial test power and sample size calculations for one-sample proportion tests. The feedback identifies several critical issues: the signature change in the _power function breaks existing calls in other modules, and the solve_size implementation for exact tests is incomplete, lacking both a return statement and the logic to determine the minimum sample size. Furthermore, the power calculations for lower and two-sided tails are currently anti-conservative and require adjustments to the critical values. Finally, the documentation for the new exact test feature needs to be populated with mathematical background.
| alpha: float, | ||
| phat: bool, | ||
| continuity_correction: bool, | ||
| exact: bool, |
There was a problem hiding this comment.
Adding exact as a required positional argument to _power breaks existing calls in solve_null_proportion and solve_proportion, which currently only pass 7 arguments. Consider providing a default value for exact to maintain backward compatibility and fix the TypeError in those functions.
| exact: bool, | |
| exact: bool = False, |
| if exact: | ||
| initial_size = solve_size(null_proportion, proportion, alternative, alpha, power, exact=False) | ||
| search_interval_lower_bound = max(floor(0.5 * initial_size), 1) | ||
| search_interval_upper_bound = min(ceil(initial_size * 1.5), SAMPLE_SIZE_SEARCH_MAX) | ||
|
|
||
| return ceil(brentq(func, 0.000001, SAMPLE_SIZE_SEARCH_MAX)) | ||
| rng = range(search_interval_lower_bound, search_interval_upper_bound) | ||
| dict_power = dict.fromkeys(rng) | ||
| for size in rng: | ||
| dict_power[size] = solve_power(null_proportion, proportion, size, alternative, alpha, exact) | ||
| else: | ||
| return ceil(brentq(func, 0.000001, SAMPLE_SIZE_SEARCH_MAX)) |
There was a problem hiding this comment.
The implementation for exact=True in solve_size is incomplete and contains a critical bug: it lacks a return statement, causing the function to return None. Additionally, the current logic populates a dictionary but does not use it to find the minimum sample size. The search logic should iterate through the range and return the smallest size that achieves the target power. Also, ensure that exact is passed correctly to solve_power (it is currently being passed as the 6th positional argument, which corresponds to phat).
| if exact: | |
| initial_size = solve_size(null_proportion, proportion, alternative, alpha, power, exact=False) | |
| search_interval_lower_bound = max(floor(0.5 * initial_size), 1) | |
| search_interval_upper_bound = min(ceil(initial_size * 1.5), SAMPLE_SIZE_SEARCH_MAX) | |
| return ceil(brentq(func, 0.000001, SAMPLE_SIZE_SEARCH_MAX)) | |
| rng = range(search_interval_lower_bound, search_interval_upper_bound) | |
| dict_power = dict.fromkeys(rng) | |
| for size in rng: | |
| dict_power[size] = solve_power(null_proportion, proportion, size, alternative, alpha, exact) | |
| else: | |
| return ceil(brentq(func, 0.000001, SAMPLE_SIZE_SEARCH_MAX)) | |
| if exact: | |
| initial_size = solve_size(null_proportion, proportion, alternative, alpha, power, exact=False) | |
| search_interval_lower_bound = max(floor(0.5 * initial_size), 1) | |
| search_interval_upper_bound = min(ceil(initial_size * 1.5), SAMPLE_SIZE_SEARCH_MAX) | |
| for size in range(search_interval_lower_bound, search_interval_upper_bound + 1): | |
| if solve_power(null_proportion, proportion, size, alternative, alpha, phat, continuity_correction, exact) >= power: | |
| return size | |
| return search_interval_upper_bound | |
| else: | |
| return ceil(brentq(func, 0.000001, SAMPLE_SIZE_SEARCH_MAX)) |
| z_alpha = binom.ppf(alpha, size, null_proportion) | ||
| power = binom.cdf(z_alpha, size, proportion) |
There was a problem hiding this comment.
The power calculation for the lower tail is anti-conservative. binom.ppf(alpha, ...) returns the smallest
| z_alpha = binom.ppf(alpha, size, null_proportion) | |
| power = binom.cdf(z_alpha, size, proportion) | |
| z_alpha = binom.ppf(alpha, size, null_proportion) | |
| if binom.cdf(z_alpha, size, null_proportion) > alpha: | |
| z_alpha -= 1 | |
| power = binom.cdf(z_alpha, size, proportion) |
| z_alpha_2 = binom.ppf(alpha / 2, size, null_proportion) | ||
| power = 1 - binom.cdf(z_alpha_1, size, proportion) + binom.cdf(z_alpha_2, size, proportion) |
There was a problem hiding this comment.
Similar to the one-sided lower tail, the two-sided test implementation might be anti-conservative because z_alpha_2 (the lower critical value) is calculated using ppf without checking if the resulting CDF exceeds
| z_alpha_2 = binom.ppf(alpha / 2, size, null_proportion) | |
| power = 1 - binom.cdf(z_alpha_1, size, proportion) + binom.cdf(z_alpha_2, size, proportion) | |
| z_alpha_2 = binom.ppf(alpha / 2, size, null_proportion) | |
| if binom.cdf(z_alpha_2, size, null_proportion) > alpha / 2: | |
| z_alpha_2 -= 1 | |
| power = 1 - binom.cdf(z_alpha_1, size, proportion) + binom.cdf(z_alpha_2, size, proportion) |
| n = \frac{n'}{4} \left( 1 + \sqrt{1 + \frac{2}{|p_1 - p_0| n'}}\right)^2 | ||
| $$ | ||
|
|
||
| ## 精确检验 {exact-test} |
No description provided.