Skip to content

Commit

Permalink
fix the TMB warning
Browse files Browse the repository at this point in the history
  • Loading branch information
danielinteractive committed Sep 27, 2024
1 parent cf72c07 commit 7b36585
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 38 deletions.
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

### Bug Fixes

- Previously, when the tape optimizer from `TMB` was switched on a warning would be given, instructing to turn off the tape optimizer. However, this is not necessary for reproducible results. Instead now it is checked whether the deterministic hash for the TMB tape optimizer is used, and a warning is issued otherwise.

# mmrm 0.3.13

### Bug Fixes

- When running with `TMB` package versions below 1.9.15, MMRM fit results are not completely reproducible. While this may not be relevant for most applications, because the numerical differences are very small, we now issue a warning to the user if this is the case. We advise users to upgrade their `TMB` package versions to 1.9.15 or higher to ensure reproducibility.
- Previously, `mmrm` ignored contrasts defined for covariates in the input data set. This is fixed now.
- Previously, `predict` always required the response to be valid, even for unconditional predictions. This is fixed now and unconditional prediction does not require the response to be valid or present any longer.
Expand Down
2 changes: 1 addition & 1 deletion R/fit.R
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ mmrm <- function(formula,
h_valid_formula(formula)
covariance <- h_reconcile_cov_struct(formula, covariance)
formula_parts <- h_mmrm_tmb_formula_parts(formula, covariance)
h_tmb_warn_optimization()
h_tmb_warn_non_deterministic()

if (!missing(data)) {
attr(data, which = "dataname") <- toString(match.call()$data)
Expand Down
18 changes: 10 additions & 8 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -524,20 +524,22 @@ h_drop_levels <- function(data, subject_var, visit_var, except) {
data
}

#' Warn if TMB is Configured to Optimize Instantly
#' Warn if TMB is Configured to Use Non-Deterministic Hash for Tape Optimizer
#'
#' This function checks the TMB configuration for the `optimize.instantly` setting.
#' If it is set to `TRUE`, a warning is issued indicating that this may lead to
#' This function checks the TMB configuration for the `tmbad_deterministic_hash` setting.
#' If it is set to `FALSE`, a warning is issued indicating that this may lead to
#' unreproducible results.
#'
#' @return No return value, called for side effects.
#' @keywords internal
h_tmb_warn_optimization <- function() {
tmb_config <- TMB::config("optimize.instantly", DLL = "mmrm")
if (tmb_config$optimize.instantly) {
h_tmb_warn_non_deterministic <- function() {
tmb_config <- TMB::config(DLL = "mmrm")
tape_deterministic <- tmb_config$tmbad_deterministic_hash
if (is.null(tape_deterministic) || !tape_deterministic) {
msg <- paste(
"TMB is configured to optimize instantly, this may lead to unreproducible results.",
"To disable this behavior, use `TMB::config(optimize.instantly = 0)`.",
"TMB is configured to use a non-deterministic hash for its tape optimizer,",
"and this may lead to unreproducible results.",
"To disable this behavior, use `TMB::config(tmbad_deterministic_hash = 1)`.",
sep = "\n"
)
warning(msg)
Expand Down
17 changes: 17 additions & 0 deletions man/h_tmb_warn_non_deterministic.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 0 additions & 17 deletions man/h_tmb_warn_optimization.Rd

This file was deleted.

4 changes: 2 additions & 2 deletions tests/testthat/helper-examples.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# We need to disable instant optimization to run tests.
TMB::config(optimize.instantly = 0, DLL = "mmrm")
# We need to make sure to have deterministic hash to run tests.
TMB::config(tmbad_deterministic_hash = 1, DLL = "mmrm")

.tmb_formula <- FEV1 ~ RACE + us(AVISIT | USUBJID)
.mmrm_tmb_example <- fit_mmrm(.tmb_formula, fev_data, weights = rep(1, nrow(fev_data)))
Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-fit.R
Original file line number Diff line number Diff line change
Expand Up @@ -935,13 +935,13 @@ test_that("mmrm fails for vcov: Jackknife and method: Kenward-Roger", {
})

test_that("mmrm gives warning if not reproducible TMB option is used", {
TMB::config(optimize.instantly = 1, DLL = "mmrm")
TMB::config(tmbad_deterministic_hash = 0, DLL = "mmrm")
expect_warning(
mmrm(
formula = FEV1 ~ ARMCD + ar1(AVISIT | USUBJID),
data = fev_data
),
"TMB is configured to optimize instantly"
"TMB is configured to use a non-deterministic hash"
)
TMB::config(optimize.instantly = 0, DLL = "mmrm")
TMB::config(tmbad_deterministic_hash = 1, DLL = "mmrm")
})

Check warning on line 947 in tests/testthat/test-fit.R

View workflow job for this annotation

GitHub Actions / SuperLinter 🦸‍♀️ / Lint R code 🧶

file=tests/testthat/test-fit.R,line=947,col=3,[trailing_blank_lines_linter] Missing terminal newline.
14 changes: 7 additions & 7 deletions tests/testthat/test-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -323,16 +323,16 @@ test_that("h_drop_levels works as expected", {
)
})

# h_tmb_warn_optimization ----
# h_tmb_warn_non_deterministic ----

test_that("h_tmb_warn_optimization works as expected", {
TMB::config(optimize.instantly = 1, DLL = "mmrm")
test_that("h_tmb_warn_non_deterministic works as expected", {
TMB::config(tmbad_deterministic_hash = 0, DLL = "mmrm")
expect_warning(
h_tmb_warn_optimization(),
"TMB is configured to optimize instantly"
h_tmb_warn_non_deterministic(),
"TMB is configured to use a non-deterministic hash"
)
TMB::config(optimize.instantly = 0, DLL = "mmrm")
expect_silent(h_tmb_warn_optimization())
TMB::config(tmbad_deterministic_hash = 1, DLL = "mmrm")
expect_silent(h_tmb_warn_non_deterministic())
})

# h_get_na_action ----
Expand Down

0 comments on commit 7b36585

Please sign in to comment.