ENH: Implement the exact p-value of the Cramér-von Mises two-sample test#108
ENH: Implement the exact p-value of the Cramér-von Mises two-sample test#108fbourgey wants to merge 3 commits intoscipy:mainfrom
Conversation
| for (int64_t u = 0; u < n + 1; ++u) { | ||
| std::vector<FreqTable> next_gs; | ||
| FreqTable tmp; | ||
| for (int64_t v = 0; v < m + 1; ++v) { | ||
| // Calculate g recursively with eq. 11 in [1]. Even though it | ||
| // doesn't look like it, this also does 12/13 (all of Algorithm 1). | ||
| const FreqTable &g = gs[v]; | ||
| // Merge tmp and g: for common keys sum frequencies, | ||
| // keep unique keys from both sides. | ||
| // (equivalent to np.intersect1d + concatenate logic) | ||
| FreqTable merged; | ||
| for (const auto &[key, freq] : tmp) { | ||
| merged[key] += freq; | ||
| } | ||
| for (const auto &[key, freq] : g) { | ||
| merged[key] += freq; | ||
| } | ||
| int64_t diff = a * v - b * u; | ||
| int64_t res = diff * diff; | ||
| // tmp[0] += res (shift all keys by res) | ||
| tmp.clear(); | ||
| for (const auto &[key, freq] : merged) { | ||
| tmp[key + res] += freq; | ||
| } | ||
| next_gs.push_back(tmp); | ||
| } | ||
| gs = std::move(next_gs); | ||
| } |
There was a problem hiding this comment.
Can you provide additional information that helps the reader compare this to either the original code, which is pretty hard to follow, or the algorithm, which looks simple but has terms like dicts, use of delete, etc.), so if it can't be translated directly, it is probably not worth trying to maintain a resemblance.
There was a problem hiding this comment.
I agree; I've found the Python code quite difficult to follow. I’ll provide more detail and include screenshots of the relevant sections of the paper in the coming days.
| using FreqTable = std::map<int64_t, int64_t>; | ||
| // the frequency table of g_{u, v}^+ defined in [1, p. 6] | ||
| // gs[0] = {0: 1}, gs[1..m] = empty | ||
| std::vector<FreqTable> gs(m + 1); |
There was a problem hiding this comment.
@mdhaber, you want this to be able to work in CuPy right? std::map and std::vector aren't available in CUDA.
Also, rather than putting this whole thing into a scalar kernel, and making a ufunc out it, it seems like it may be better to take the idea of the original function and decompose it into the ufuncs that are needed to make it work. I think the key component here would be a gufunc to generate the frequency table. lcm and the binomial coefficients could be handled through delegation, and the rest looks like it could be made array-agnostic.
There was a problem hiding this comment.
you want this to be able to work in CuPy right?
Yes. OK, good to know.
| */ | ||
| inline double pval_cvm_2samp_exact(double s, int64_t m, int64_t n) { | ||
| // [1, p. 3] | ||
| int64_t lcm = std::lcm(m, n); |
| int64_t a = lcm / m; | ||
| int64_t b = lcm / n; |
| // Combine Eq. 9 in [2] with Eq. 2 in [1] and solve for $\zeta$ | ||
| // Hint: `s` is $U$ in [2], and $T_2$ in [1] is $T$ in [2] | ||
| int64_t mn = m * n; | ||
| // Uses double floor division since s is double | ||
| int64_t zeta = std::floor((lcm * lcm * (m + n) * (6.0 * s - mn * (4.0 * mn - 1))) / (6.0 * mn * mn)); |
There was a problem hiding this comment.
Using consistent notations and following the Python comment:
From [1, page 3]
Rewriting in terms of
and from [2, eq. 9] (and using the same notations as in [1])
Simple algebra gives
We then floor as
| std::vector<FreqTable> gs(m + 1); | ||
| gs[0][0] = 1; |
There was a problem hiding this comment.
Initialize the frequency tables of
Each table is a
g = [[zeta_1, zeta_2, ..., zeta_k],
[count_1, count_2, ..., count_k]]row 0 = distinct values of
Initially:
| // (equivalent to return np.float64(np.sum(freq[value >= zeta]) / combinations)) | ||
| const FreqTable &final_table = gs[m]; | ||
| int64_t sum_freq = 0; | ||
| for (const auto &[value, freq] : final_table) { | ||
| if (value >= zeta) { | ||
| sum_freq += freq; | ||
| } | ||
| } | ||
| return static_cast<double>(sum_freq) / static_cast<double>(combinations); | ||
| } |
There was a problem hiding this comment.
Compute p-value
| int64_t diff = a * v - b * u; | ||
| int64_t res = diff * diff; | ||
| // tmp[0] += res (shift all keys by res) | ||
| tmp.clear(); | ||
| for (const auto &[key, freq] : merged) { | ||
| tmp[key + res] += freq; | ||
| } |
| const FreqTable &g = gs[v]; | ||
| // Merge tmp and g: for common keys sum frequencies, | ||
| // keep unique keys from both sides. | ||
| // (equivalent to np.intersect1d + concatenate logic) | ||
| FreqTable merged; | ||
| for (const auto &[key, freq] : tmp) { | ||
| merged[key] += freq; | ||
| } | ||
| for (const auto &[key, freq] : g) { | ||
| merged[key] += freq; | ||
| } |
There was a problem hiding this comment.
Translation of the original Python code.
vi, i0, i1 = np.intersect1d(tmp[0], g[0], return_indices=True)
tmp = np.concatenate([
np.stack([vi, tmp[1, i0] + g[1, i1]]),
np.delete(tmp, i0, 1),
np.delete(g, i1, 1)
], 1)- First line: find common
$\zeta$ values between$g_{u,v-1}$ and$g_{u-1,v}$ - Second line: this implements
$g_{u,v-1} + g_{u-1,v}$
| FreqTable tmp; | ||
| for (int64_t v = 0; v < m + 1; ++v) { | ||
| // Calculate g recursively with eq. 11 in [1]. Even though it | ||
| // doesn't look like it, this also does 12/13 (all of Algorithm 1). |
There was a problem hiding this comment.
It appears that (12) is not needed, as this is produced automatically by how gs and tmp are initialized.
For
Starting from
Here, we use that


Reference issue
Toward #98
What does this implement/fix?
Implement the exact p-value of the Cramer-von Mises two-sample test for a given value of the test statistic from
_pval_cvm_2samp_exacthttps://en.wikipedia.org/wiki/Cram%C3%A9r%E2%80%93von_Mises_criterion