fixed the truncation operation for the POT-C & PRIT-C uniformity test.

[hermanFTT]

Closes #461

Summary

1. Truncation operation of POT-C and PRIT-C Cauchy combinaision test of Tesso & Vehtari currently implemented with "indexing-then-mean" operation have been replaced to align with the actual paper formulation. The mean operation should be over the total number of pointwise p-values instead of over the length of the subset of p-values greater than 0.5 ( see issue Cauchy combination tests: truncation operation inconsistent with paper specification. #461 ).

2. An adjustment for PRIT-C based on mid p-values correction is suggested to avoid p-values greater than 1.

3. A calibration unit test is also included in the test folder.

AI disclosure

No AI assistance used for this PR.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 2d89594 is merged into master:

• ✔️as_draws_array: 148ms -> 147ms [-2.06%, +0.88%]
• ✔️as_draws_df: 71.3ms -> 71.4ms [-2.17%, +2.5%]
• ✔️as_draws_list: 147ms -> 148ms [-1.42%, +2.13%]
• ✔️as_draws_matrix: 24.6ms -> 25ms [-0.13%, +3.67%]
• ✔️as_draws_rvars: 116ms -> 117ms [-1%, +1.77%]
• ✔️summarise_draws_100_variables: 730ms -> 731ms [-1.23%, +1.54%]
• ✔️summarise_draws_10_variables: 80.6ms -> 80.7ms [-0.35%, +0.54%]
  Further explanation regarding interpretation and methodology can be found in the documentation.

[florence-bockting]

Thank you very much for the PR @hermanFTT. It looks very good, I have only minor changes which are mainly styling aspects (indentation, white spaces, new lines, etc.)

Otherwise it looks very good and I think after this small changes it is good to go from my side.

Tagging @paul-buerkner for having a short second look at it (specifically regarding the calibration test)

Thank you!

[florence-bockting]

It would be nice if you @hermanFTT could additionally provide a short information about your change in the NEWS.md
Simply add a new section like

# posterior (development)
### Bug Fixes

* description of your change by @hermanFTT (#462)

# posterior 1.7.1
...

[paul-buerkner]

I am a bit out of the code for the uniformity tests right now. @avehtari can you check? I assume this PR was pre-discussed with you?

[avehtari]

• I agree on skipping slow test on CRAN
• maybe add tests for the new no-p<0.5 → 0.5 behavior and for truncate=FALSE to improve coverage?

[florence-bockting]

• I agree on skipping slow test on CRAN
• maybe add tests for the new no-p<0.5 → 0.5 behavior and for truncate=FALSE to improve coverage?

Thank you for your review!

@hermanFTT do you want to give it a try and suggest follow-up tests?
Regarding the suggested code changes: The way how this usually works, is that you go over the suggestions, and when you are "fine" with them, then you click "Apply suggestion". A drop-down window opens and you can provide a text which will be used as commit message once you click "commit changes". This will automatically commit to the branch. You can then locally on your computer "pull" the changes and then you will directly see the code changes.

Regarding the order, I would suggest,

1. you go first over my code suggestions and decide for each whether you want to apply the change or not.
2. Then you change to your local setup and use "git pull" to update your local branch.
3. Then you write the new tests (according to Aki's suggestion) and edit the NEWS.md.
4. Then git add, commit, and push the changes and tag (@florence-bockting) me again.

When you are uncertain or have any question in one of these steps, don't hesitate and let me know.

[hermanFTT]

@florence-bockting I have reviewed and updated the PR based on the review/suggestions. See, e.g, additional unit test for the case truncated = TRUE when all pointwise $p$-values to be combined are above 0.5 .

[florence-bockting]

@florence-bockting I have reviewed and updated the PR based on the review/suggestions. See, e.g, additional unit test for the case truncated = TRUE when all pointwise p -values to be combined are above 0.5 .

Perfect, looks very good. Thank you @hermanFTT.
What do you think about adding the following two tests which address explicitly the point of using discrete values for PRIT? I think, then we can merge the PR.

test_that("PRIT is calibrated for discrete pit values", {
  skip_on_cran()
  set.seed(4711)
  nsim <- 1000

  draw_pit <- function() pbinom(rbinom(100, 5, 0.5), 5, 0.5)
  pvals <- replicate(nsim, uniformity_test(draw_pit(), "PRIT")$pvalue)
  res <- mean(pvals < 0.01)
  expect_equal(res, 0.01, tolerance = 1e-2)
})

test_that("prit_test gives equal p-values to tied PIT values", {
  # Let n = 4, x = c(0.25, 0.25, 0.75, 0.75)
  # scaled_ecdf = 4 * c(0.5, 0.5, 1, 1) = c(2, 2, 4, 4)
  # For x = 0.25: pbinom(2, 4, 0.25) = 0.94921875
  #               dbinom(2, 4, 0.25) = 0.2109375
  #               probs1 = 0.94921875 - 0.10546875 = 0.84375
  #               p_val = 2 * (1 - 0.84375) = 0.3125
  # For x = 0.75: pbinom(4, 4, 0.75) = 1
  #               dbinom(4, 4, 0.75) = 0.31640625
  #               probs1 = 1 - 0.158203125 = 0.841796875
  #               p_val = 2 * 0.158203125 = 0.31640625

  x <- c(0.25, 0.25, 0.75, 0.75)
  expected <- c(0.3125, 0.3125, 0.31640625, 0.31640625)

  expect_equal(.prit_test(x), expected, tolerance = 1e-10)
})

[hermanFTT]

I actually had similar tests but somehow forgot to commit and push. It is now updated.

[paul-buerkner]

Thanks! @florence-bockting you can then merge whenever you feel like (I assume you have the rights?)