ENH: Implement cvm_cdf_inf from scipy.stats._hypotests#97
ENH: Implement cvm_cdf_inf from scipy.stats._hypotests#97fbourgey wants to merge 3 commits intoscipy:mainfrom
Conversation
There was a problem hiding this comment.
Thanks @fbourgey!
@steppi I have a few questions about preferred style in xsf, but I checked this from a content perspective, and it LGTM. It's a pretty direct translation of the Python code, and the test compares the results against those from Python. I assume the CI failure is unrelated, so I think it's ready for your review.
| if (std::abs(z) < 1e-7) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Does xsf have any tools for summing infinite series like SciPy/mpmath's nsum?
tests/xsf_tests/test_cdf_cvm_inf.cpp
Outdated
| // Generate linspace: xs = np.linspace(2e-3, 1 - 2e-3, 51) | ||
| const int n_points = 51; | ||
| const double start = 2e-3; | ||
| const double end = 1.0 - 2e-3; | ||
| std::vector<double> xs(n_points); | ||
| for (int i = 0; i < n_points; ++i) { | ||
| xs[i] = start + (end - start) * i / (n_points - 1); | ||
| } | ||
|
|
||
| // Reference values computed with scipy.stats._hypotests._cdf_cvm_inf |
There was a problem hiding this comment.
Would it be good to just include the complete Python code? That makes it easier to verify that the numbers are correct.
// Reference values computed with scipy.stats._hypotests._cdf_cvm_inf
// from scipy.stats._hypotests import _cdf_cvm_inf
// x = np.linspace(2e-3, 1-2e-3, 51)
// res = _cdf_cvm_inf(x)
There was a problem hiding this comment.
I've added the suggestion.
|
|
||
| // Reference values computed with scipy.stats._hypotests._cdf_cvm_inf | ||
| const std::vector<double> expected = { | ||
| 1.14362132e-27, 5.17604145e-03, 7.65622444e-02, 1.97103480e-01, 3.17803769e-01, 4.22913972e-01, 5.10702567e-01, |
There was a problem hiding this comment.
Is it preferred in xsf to truncate the results to a certain number of digits? In any case, these are the right numbers.
|
|
||
| const double rtol = 1e-8; | ||
|
|
||
| for (int i = 0; i < n_points; ++i) { |
There was a problem hiding this comment.
@steppi your eyes will be more attuned to the standard testing idioms, but this looks like it's comparing the right things.
|
|
||
| inline double chdtri(double df, double y) { return cephes::chdtri(df, y); } | ||
|
|
||
| inline double cvm_cdf_inf(double x) { |
There was a problem hiding this comment.
This looks like a faithful translation of the original code.
2 participants
Following @mdhaber's comment and @steppi's suggestion:
Port the asymptotic (infinite sample size) null distribution CDF of the single-sample Cramér-von Mises statistic
_cvm_cdf_inffromscipy.stats._hypoteststoxsf/stats.hI've added a test with reference values computed from
scipy.stats._hypotests._cvm_cdf_infFor reference, this is based on the asymptotic formula from Csörgő, S. and Faraway, J. (1996).