Issue 806: warn about nonparametric generation times (#808)

* deprecate nonzero 0-bin generation times

* add deprecation test

* clarify documentation

* add news item

NEWS.md

@@ -33,6 +33,7 @@ A release that introduces model improvements to the Gaussian Process models, alo
 - `fix_dist()` has been renamed to `fix_parameters()` because it removes the uncertainty in a distribution's parameters. By @sbfnk in #733 and reviewed by @jamesmbaazam.
 - `plot.dist_spec` now uses color instead of line types to display pmfs vs cmfs. By @jamesmbaazam in #788 and reviewed by @sbfnk.
 - The use of the `{progressr}` package for displaying progress bars is now optional, as is the use of `{future}` and `{future.apply}` for parallelisation. By @sbfnk in #798 and reviewed by @seabbs.
+- Specifying nonparametric generation time intervals with a nonzero first element (corresponding to the zero bin) is being deprecated as the current behaviour of setting it to zero internally was not well exposed. By @sbfnk in #.
 
 ## Bug fixes
 

R/checks.R

@@ -120,6 +120,39 @@ check_stan_delay <- function(dist) {
   }
 }
 
+#' Validate probability distribution for using as generation time
+#'
+#' @description `r lifecycle::badge("stable")`
+#' does all the checks in`check_stan_delay()` and additionally makes sure
+#' that if `dist` is nonparametric,  its first element is zero.
+#'
+#' @importFrom lifecycle deprecate_warn
+#' @inheritParams check_stan_delay dist
+#' @return Called for its side effects.
+#' @keywords internal
+check_generation_time <- function(dist) {
+  # Do the standard delay checks
+  check_stan_delay(dist)
+  ## check for nonparametric with nonzero first element
+  nonzero_first_element <- vapply(seq_len(ndist(dist)), function(i) {
+    get_distribution(dist, i) == "nonparametric" && get_pmf(dist, i)[1] > 0
+  }, logical(1))
+  if (all(nonzero_first_element)) {
+    deprecate_warn(
+      "1.6.0",
+      I(
+        "Specifying nonparametric generation times with nonzero first element"
+      ),
+      details = c(
+        "Since zero generation times are not supported by the model, the
+         generation time will be left-truncated at one. ",
+        "In future versions this will cause an error. Please ensure that the
+         first element of the nonparametric generation interval is zero."
+      )
+    )
+  }
+}
+
 #' Check that PMF tail is not sparse
 #'
 #' @description Checks if the tail of a PMF vector has more than `span`

R/opts.R

@@ -3,9 +3,16 @@
 #' @description `r lifecycle::badge("stable")`
 #' Returns generation time parameters in a format for lower level model use.
 #'
+#' @details Because the discretised renewal equation used in the package does
+#'   not support zero generation times, any distribution specified here will be
+#'   left-truncated at one, i.e. the first element of the nonparametric or
+#'   discretised probability distribution used for the generation time is set to
+#'   zero and the resulting distribution renormalised.
 #' @rdname generation_time_opts
 #' @param dist A delay distribution or series of delay distributions . If no
-#'   distribution is given a fixed generation time of 1 will be assumed.
+#'   distribution is given a fixed generation time of 1 will be assumed.  If
+#'   passing a nonparametric distribution the first element should be zero (see
+#'   *Details* section)
 #'
 #' @param ... deprecated; use `dist` instead
 #' @param disease deprecated; use `dist` instead
@@ -83,7 +90,7 @@ gt_opts <- function(dist = Fixed(1), ...,
   )
   attr(dist, "weight_prior") <- weight_prior
   attr(dist, "class") <- c("generation_time_opts", class(dist))
-  check_stan_delay(dist)
+  check_generation_time(dist)
   return(dist)
 }
 

tests/testthat/test-delays.R

@@ -92,3 +92,13 @@ test_that("distributions incompatible with stan models are caught", {
     Normal(2, 2, max = 10)
   ), "lognormal")
 })
+
+test_that("nonparametric PMFs with nonzero first element for the generation time are deprecated", {
+  expect_deprecated(
+    gt_opts(c(
+      NonParametric(c(0.1, 0.5, 0.4)),
+      NonParametric(c(0.2, 0.6, 0.2))
+    )),
+    "nonzero first element"
+  )
+})