Skip to content

Commit

Permalink
Merge pull request #402 from mrc-ide/false-convergence-warning-to-error
Browse files Browse the repository at this point in the history
False convergence warning to error
  • Loading branch information
r-ash authored Mar 26, 2024
2 parents 9a51c45 + 4145443 commit 6a8e9c6
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 20 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: naomi
Title: Naomi Model for Subnational HIV Estimates
Version: 2.9.25
Version: 2.9.26
Authors@R:
person(given = "Jeff",
family = "Eaton",
Expand Down
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@

# naomi 2.9.26

* Change wording of false convergence error to more informative warning.

# naomi 2.9.25

* Suppress warning raised from duckdb shutdown
Expand All @@ -8,6 +13,7 @@
* Fix R CMD check notes
* Unpin duckdb version


# naomi 2.9.23

* Update Datim UIDs for Ethiopia 2024 boundary division.
Expand Down
14 changes: 11 additions & 3 deletions R/run-model.R
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,17 @@ run_model <- function(data, options, validate) {
max_iter = options$max_iterations %||% 250,
progress = progress)

if(fit$convergence != 0) {
naomi_warning(t_("WARNING_CONVERGENCE", list(msg = fit$message)),
"model_fit")

if (fit$convergence !=0) {

if(fit$message == "false convergence (8)"){
msg <- t_("WARNING_FALSE_CONVERGENCE")
} else {
msg <- t_("WARNING_CONVERGENCE", list(msg = fit$message))
}

naomi_warning(msg, "model_fit")

}

progress$finalise_fit()
Expand Down
5 changes: 3 additions & 2 deletions R/tmb-model.R
Original file line number Diff line number Diff line change
Expand Up @@ -588,14 +588,15 @@ fit_tmb <- function(tmb_input,
obj <- make_tmb_obj(tmb_input$data, tmb_input$par_init, calc_outputs = 0L,
inner_verbose, progress)

trace <- if(outer_verbose) 1 else 0
trace <- if (outer_verbose) 1 else 0
f <- withCallingHandlers(
stats::nlminb(obj$par, obj$fn, obj$gr,
control = list(trace = trace,
iter.max = max_iter)),
warning = function(w) {
if(grepl("NA/NaN function evaluation", w$message))
if (grepl("NA/NaN function evaluation", w$message)) {
invokeRestart("muffleWarning")
}
}
)

Expand Down
1 change: 1 addition & 0 deletions inst/traduire/en-translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@
"WARNING_OUTPUTS_PREV_EXCEEDS_THRESHOLD": "HIV prevalence is higher than 50% for: {{rows}}",
"WARNING_OUTPUTS_ARTCOV_EXCEEDS_THRESHOLD": "ART coverage is higher than 100% for: {{rows}}",
"WARNING_CONVERGENCE": "Convergence error: {{msg}}",
"WARNING_FALSE_CONVERGENCE": "Model fitting to input data has not fully converged. Please review estimates of HIV prevalence and ART coverage across districts and the national distribution of key indicators by age and sex.",
"NAVIGATOR_ART_IS_SPECTRUM_DESC": "Total on ART in Naomi matches Spectrum ART",
"NAVIGATOR_ANC_IS_SPECTRUM_DESC": "Naomi num ANC clients 2015 to present is Spectrum ANC testing cascade",
"NAVIGATOR_PACKAGE_CREATED_DESC": "Naomi package created",
Expand Down
1 change: 1 addition & 0 deletions inst/traduire/fr-translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
"WARNING_OUTPUTS_PREV_EXCEEDS_THRESHOLD": "La prévalence du VIH est supérieure à 50% pour: {{rows}}",
"WARNING_OUTPUTS_ARTCOV_EXCEEDS_THRESHOLD": "La couverture du traitement antirétroviral est supérieure à 100 % pour: {{rows}}",
"WARNING_CONVERGENCE": "Erreur de convergence: {{msg}}",
"WARNING_FALSE_CONVERGENCE": "L'ajustement du modèle aux données d'entrée n'a pas complètement convergé. Veuillez revoir les estimations de la prévalence du VIH et de la couverture des traitements antirétroviraux dans les districts, ainsi que la distribution nationale des indicateurs clés par âge et par sexe.",
"POPULATION_RATIO": "La population proportion",
"PLHIV_RATIO": "PVVIH proportion",
"NAVIGATOR_ANC_IS_SPECTRUM_DESC": "Naomi nombre de clients CPN 2015 à aujourd'hui est Spectrum cascade de test CPN",
Expand Down
1 change: 1 addition & 0 deletions inst/traduire/pt-translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
"WARNING_OUTPUTS_PREV_EXCEEDS_THRESHOLD": "A prevalência do VIH é superior a 50% para: {{rows}}",
"WARNING_OUTPUTS_ARTCOV_EXCEEDS_THRESHOLD": "A cobertura TARV é superior a 100% para: {{rows}}",
"WARNING_CONVERGENCE": "Erro de convergência: {{msg}}",
"WARNING_FALSE_CONVERGENCE": "O ajuste do modelo aos dados de entrada não convergiu totalmente. Rever as estimativas da prevalência do VIH e da cobertura de TARV nos distritos e a distribuição nacional dos principais indicadores por idade e sexo.",
"NAVIGATOR_ANC_IS_SPECTRUM_DESC": "Naomi número de clientes de ANC 2015 a apresentar é Spectrum cascata de testes de ANC",
"NAVIGATOR_ART_IS_SPECTRUM_DESC": "O total em TARV em Naomi corresponde ao Spectrum TARV",
"NAVIGATOR_PACKAGE_CREATED_DESC": "Pacote Naomi criado",
Expand Down
4 changes: 0 additions & 4 deletions tests/testthat/test-03-model-fit.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ test_that("setting different rng_seed returns different output", {
)
})

test_that("exceeding maximum iterations throws a warning", {
expect_warning(fit_tmb(a_tmb_inputs, outer_verbose = FALSE, max_iter = 5),
"iteration limit reached")
})

test_that("model fits with differing number of ANC observations T1 and T2", {

Expand Down
17 changes: 7 additions & 10 deletions tests/testthat/test-warning.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,19 @@ test_that("naomi warning handler returns empty list when no warnings", {
expect_length(out$warnings, 0)
})


test_that("warning raised after false convergence", {

a_fit_bad <- a_fit
a_fit_bad$convergence <- 1
a_fit_bad$message <- "false convergence (8)"

mock_fit_tmb <- mockery::mock(a_fit_bad)
mock_sample_tmb <- mockery::mock(a_fit_sample)
mock_output_package <- mockery::mock(a_output)

with_mock(
fit_tmb = mock_fit_tmb,
sample_tmb = mock_sample_tmb,
output_package = mock_output_package, {
fit_tmb = mockery::mock(a_fit_bad),
sample_tmb = mockery::mock(a_fit_sample),
output_package = mockery::mock(a_output), {
out <- hintr_run_model(a_hintr_data, a_hintr_options, validate = FALSE)
})
}
)

expect_length(out$warnings, 4)
expect_match(out$warnings[[1]]$text,
Expand All @@ -69,7 +66,7 @@ test_that("warning raised after false convergence", {
expect_match(out$warnings[[3]]$text,
"Naomi ANC tested positive not equal to Spectrum")
expect_equal(out$warnings[[4]]$text,
"Convergence error: false convergence (8)")
"Model fitting to input data has not fully converged. Please review estimates of HIV prevalence and ART coverage across districts and the national distribution of key indicators by age and sex.")
})


Expand Down

0 comments on commit 6a8e9c6

Please sign in to comment.