Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False convergence warning to error #402

Merged
merged 11 commits into from
Mar 26, 2024
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
Loading