From c881c9ba001d9ac68fbe51e6017ec6c4d3a1eba0 Mon Sep 17 00:00:00 2001
From: jgabry <jgabry@gmail.com>
Date: Sun, 25 Feb 2024 14:35:15 -0700
Subject: [PATCH] minor updates to doc and tests after latest loo release

---
 R/fit.R                        |  7 ++++---
 man/fit-method-loo.Rd          |  5 +++--
 tests/testthat/test-fit-mcmc.R | 29 +++++++++++++++++++----------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/R/fit.R b/R/fit.R
index 554a79b32..cc1e2e53e 100644
--- a/R/fit.R
+++ b/R/fit.R
@@ -312,7 +312,7 @@ CmdStanFit$set("public", name = "init", value = init)
 #' @description The `$init_model_methods()` method compiles and initializes the
 #'   `log_prob`, `grad_log_prob`, `constrain_variables`, `unconstrain_variables`
 #'   and `unconstrain_draws` functions. These are then available as methods of
-#'   the fitted model object. This requires the additional `Rcpp` package, 
+#'   the fitted model object. This requires the additional `Rcpp` package,
 #'   which are not required for fitting models using
 #'   CmdStanR.
 #'
@@ -1483,8 +1483,9 @@ CmdStanMCMC <- R6::R6Class(
 #' @param r_eff (multiple options) How to handle the `r_eff` argument for `loo()`:
 #'   * `TRUE` (the default) will automatically call [loo::relative_eff.array()]
 #'   to compute the `r_eff` argument to pass to [loo::loo.array()].
-#'   * `FALSE` or `NULL` will avoid computing `r_eff` (which can sometimes be slow)
-#'   but will result in a warning from the \pkg{loo} package.
+#'   * `FALSE` or `NULL` will avoid computing `r_eff` (which can sometimes be slow),
+#'   but the reported ESS and MCSE estimates can be over-optimistic if the
+#'   posterior draws are not (near) independent.
 #'   * If `r_eff` is anything else, that object will be passed as the `r_eff`
 #'   argument to [loo::loo.array()].
 #' @param moment_match (logical) Whether to use a
diff --git a/man/fit-method-loo.Rd b/man/fit-method-loo.Rd
index 7b84ffde5..c6d98844a 100644
--- a/man/fit-method-loo.Rd
+++ b/man/fit-method-loo.Rd
@@ -17,8 +17,9 @@ look for \code{"log_lik"}. This argument is passed to the
 \itemize{
 \item \code{TRUE} (the default) will automatically call \code{\link[loo:relative_eff]{loo::relative_eff.array()}}
 to compute the \code{r_eff} argument to pass to \code{\link[loo:loo]{loo::loo.array()}}.
-\item \code{FALSE} or \code{NULL} will avoid computing \code{r_eff} (which can sometimes be slow)
-but will result in a warning from the \pkg{loo} package.
+\item \code{FALSE} or \code{NULL} will avoid computing \code{r_eff} (which can sometimes be slow),
+but the reported ESS and MCSE estimates can be over-optimistic if the
+posterior draws are not (near) independent.
 \item If \code{r_eff} is anything else, that object will be passed as the \code{r_eff}
 argument to \code{\link[loo:loo]{loo::loo.array()}}.
 }}
diff --git a/tests/testthat/test-fit-mcmc.R b/tests/testthat/test-fit-mcmc.R
index fa33b70b6..122dc5037 100644
--- a/tests/testthat/test-fit-mcmc.R
+++ b/tests/testthat/test-fit-mcmc.R
@@ -279,19 +279,28 @@ test_that("loo method works with moment-matching", {
   # Moment-matching needs model-methods, so make sure hpp is available
   mod <- cmdstan_model(testing_stan_file("loo_moment_match"), force_recompile = TRUE)
   data_list <- testing_data("loo_moment_match")
-  fit <- mod$sample(data = data_list, chains = 1)
+  fit <- mod$sample(data = data_list, chains = 1, seed = 1000)
 
-  # Regular LOO should warn that some pareto-k are "too high"
-  expect_warning(fit$loo(),
-                  "Some Pareto k diagnostic values are too high. See help('pareto-k-diagnostic') for details.",
-                  fixed=TRUE)
+  # Regular loo should warn that some pareto-k are "too high"
+  expect_warning(
+    fit$loo(),
+    "Some Pareto k diagnostic values are too high.",
+    fixed = TRUE
+  )
 
-  # After moment-matching the warning should be downgraded to "slightly high"
-  expect_warning(fit$loo(moment_match = TRUE),
-                  "Some Pareto k diagnostic values are slightly high. See help('pareto-k-diagnostic') for details.",
-                  fixed=TRUE)
+  # In loo < 2.7.0 after moment-matching the warning should be downgraded to "slightly high"
+  if (utils::packageVersion("loo") < "2.7.0") {
+    expect_warning(
+      fit$loo(moment_match = TRUE),
+      "Some Pareto k diagnostic values are slightly high.",
+      fixed = TRUE
+    )
+  } else {
+    # But in loo >= 2.7.0 there is no "slightly high" so no warning here
+    expect_no_warning(fit$loo(moment_match = TRUE))
+  }
 
-  # After moment-matching with lower target threshold there should be no warning
+  # After moment-matching with lower target threshold there definitely shouldn't be a warning
   expect_no_warning(fit$loo(moment_match = TRUE, k_threshold=0.4))
 })