From 089f38818abc32bee8d90513e2a927f2097735bb Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:26:28 +0200 Subject: [PATCH 01/16] Try to get more info on failure --- R/extract_param.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/extract_param.R b/R/extract_param.R index 965cb0fd8..55a37727f 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -228,14 +228,16 @@ extract_param <- function(type = c("percentiles", "range"), lower <- c(-1e5, 1e-10) } - optim_params <- stats::optim( + optim_params <- tryCatch(stats::optim( param, fit_func, method = "L-BFGS-B", val = values_in, dist = distribution, lower = lower - ) + ), error = function(e) { + stop("found inf value in ", distribution) + }) optim_params } From 1adcd95ec532d1feb83d6a3328909b651521a324 Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:57:21 +0200 Subject: [PATCH 02/16] Check if Inf occurs on lower or upper --- R/extract_param.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/extract_param.R b/R/extract_param.R index 55a37727f..16f9857af 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -214,18 +214,22 @@ extract_param <- function(type = c("percentiles", "range"), if (distribution == "lnorm") { names(param) <- c("meanlog", "sdlog") lower <- c(-1e5, 1e-10) + upper <- c(1e10, 1e10) } if (distribution == "gamma") { names(param) <- c("shape", "scale") lower <- c(1e-10, 1e-10) + upper <- c(1e10, 1e10) } if (distribution == "weibull") { names(param) <- c("shape", "scale") lower <- c(1e-10, 1e-10) + upper <- c(1e10, 1e10) } if (distribution == "norm") { names(param) <- c("mean", "sd") lower <- c(-1e5, 1e-10) + upper <- c(1e10, 1e10) } optim_params <- tryCatch(stats::optim( From a9bae5c78be08e4a22a2ea3af2ee5e374cac0d03 Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Mon, 29 Apr 2024 21:02:58 +0200 Subject: [PATCH 03/16] Actually use upper --- R/extract_param.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index 16f9857af..54ca082d5 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -238,7 +238,8 @@ extract_param <- function(type = c("percentiles", "range"), method = "L-BFGS-B", val = values_in, dist = distribution, - lower = lower + lower = lower, + upper = upper ), error = function(e) { stop("found inf value in ", distribution) }) From 1c18b6e22f194f60b6b521781b54d15021528db5 Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Tue, 30 Apr 2024 09:38:52 +0200 Subject: [PATCH 04/16] Increase weibull lower bounds --- R/extract_param.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index 54ca082d5..293b1a1ed 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -223,7 +223,7 @@ extract_param <- function(type = c("percentiles", "range"), } if (distribution == "weibull") { names(param) <- c("shape", "scale") - lower <- c(1e-10, 1e-10) + lower <- c(1e-7, 1e-7) upper <- c(1e10, 1e10) } if (distribution == "norm") { From b97f0c7de6cb494d7ef4ca5706e30993bd09301f Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Tue, 30 Apr 2024 09:53:02 +0200 Subject: [PATCH 05/16] Increase lower even more? --- R/extract_param.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index 293b1a1ed..b286680fb 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -223,7 +223,7 @@ extract_param <- function(type = c("percentiles", "range"), } if (distribution == "weibull") { names(param) <- c("shape", "scale") - lower <- c(1e-7, 1e-7) + lower <- c(1e-3, 1e-3) upper <- c(1e10, 1e10) } if (distribution == "norm") { From 82df9739a06a8e04b6ffb98de0cf853cda90b952 Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:23:36 +0200 Subject: [PATCH 06/16] Adjust bounds --- R/extract_param.R | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/R/extract_param.R b/R/extract_param.R index b286680fb..a5d007a0b 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -214,22 +214,18 @@ extract_param <- function(type = c("percentiles", "range"), if (distribution == "lnorm") { names(param) <- c("meanlog", "sdlog") lower <- c(-1e5, 1e-10) - upper <- c(1e10, 1e10) } if (distribution == "gamma") { names(param) <- c("shape", "scale") lower <- c(1e-10, 1e-10) - upper <- c(1e10, 1e10) } if (distribution == "weibull") { names(param) <- c("shape", "scale") - lower <- c(1e-3, 1e-3) - upper <- c(1e10, 1e10) + lower <- c(1e-5, 1e-5) } if (distribution == "norm") { names(param) <- c("mean", "sd") lower <- c(-1e5, 1e-10) - upper <- c(1e10, 1e10) } optim_params <- tryCatch(stats::optim( @@ -238,8 +234,7 @@ extract_param <- function(type = c("percentiles", "range"), method = "L-BFGS-B", val = values_in, dist = distribution, - lower = lower, - upper = upper + lower = lower ), error = function(e) { stop("found inf value in ", distribution) }) From 7c7798374ca6f1b0cbdd24f6a6aa2ec900ee71e0 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:06:36 +0100 Subject: [PATCH 07/16] remove tryCatch in extract_param --- R/extract_param.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/R/extract_param.R b/R/extract_param.R index a5d007a0b..deb9f36a4 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -228,16 +228,14 @@ extract_param <- function(type = c("percentiles", "range"), lower <- c(-1e5, 1e-10) } - optim_params <- tryCatch(stats::optim( + optim_params <- stats::optim( param, fit_func, method = "L-BFGS-B", val = values_in, dist = distribution, lower = lower - ), error = function(e) { - stop("found inf value in ", distribution) - }) + ) optim_params } From 4143bed614aa73d0047569ed59b6de93e5404c77 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:17:07 +0100 Subject: [PATCH 08/16] print parameter index in extract-bias vignette --- vignettes/extract-bias.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/extract-bias.Rmd b/vignettes/extract-bias.Rmd index b89e7f4bf..67e7aca90 100644 --- a/vignettes/extract-bias.Rmd +++ b/vignettes/extract-bias.Rmd @@ -206,7 +206,7 @@ estim_params <- vector("list", nrow(parameters_range)) for (params_idx in seq_len(nrow(parameters_range))) { dist <- as.character(parameters_range[params_idx, "dist"]) n_samples <- parameters_range[params_idx, "n_samples"] - + print(params_idx) if (dist == "lnorm") { true_median <- do.call( paste0("q", dist), From 73fd8908d417e700b2a457aaf4898aae231e537c Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:25:17 +0100 Subject: [PATCH 09/16] add stop call to extract_param --- R/extract_param.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/extract_param.R b/R/extract_param.R index deb9f36a4..8fb0e5e8f 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -81,6 +81,11 @@ extract_param <- function(type = c("percentiles", "range"), if (distribution == "norm") { checkmate::assert_numeric(values) } else { + + if (any(values <= 1e-10)) { + stop("Distribution is ", distribution, " Values are ", values, " Samples are ", samples) + } + checkmate::assert_numeric(values, lower = 1e-10) } From 9fe4c10d0198a6b382c4d7cf1dd9b2d16f35e530 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:29:52 +0100 Subject: [PATCH 10/16] remove print from vignette and change lower bound in extract_param value assert to 0 --- R/extract_param.R | 8 ++++---- vignettes/extract-bias.Rmd | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/R/extract_param.R b/R/extract_param.R index 8fb0e5e8f..b598b6e0b 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -82,11 +82,11 @@ extract_param <- function(type = c("percentiles", "range"), checkmate::assert_numeric(values) } else { - if (any(values <= 1e-10)) { - stop("Distribution is ", distribution, " Values are ", values, " Samples are ", samples) - } + # if (any(values <= 1e-10)) { + # stop("Distribution is ", distribution, " Values are ", values, " Samples are ", samples) + # } - checkmate::assert_numeric(values, lower = 1e-10) + checkmate::assert_numeric(values, lower = 0) } if (type == "percentiles") { diff --git a/vignettes/extract-bias.Rmd b/vignettes/extract-bias.Rmd index 67e7aca90..cbe422314 100644 --- a/vignettes/extract-bias.Rmd +++ b/vignettes/extract-bias.Rmd @@ -206,7 +206,6 @@ estim_params <- vector("list", nrow(parameters_range)) for (params_idx in seq_len(nrow(parameters_range))) { dist <- as.character(parameters_range[params_idx, "dist"]) n_samples <- parameters_range[params_idx, "n_samples"] - print(params_idx) if (dist == "lnorm") { true_median <- do.call( paste0("q", dist), From 3bb0e0d1dba04a381ac43a89feea5f55f2a85ee4 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:37:20 +0100 Subject: [PATCH 11/16] add stop to extract_param to check values --- R/extract_param.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/R/extract_param.R b/R/extract_param.R index b598b6e0b..3819d8479 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -100,6 +100,11 @@ extract_param <- function(type = c("percentiles", "range"), checkmate::assert_numeric(percentiles, lower = 0, upper = 1, len = 2) } if (identical(type, "range")) { + + if (values[2] < values[1] && values[1] < values[3]) { + stop("Distribution is ", distribution, " Values are ", values, " Type is ", type) + } + stopifnot( "samples need to be given for type = 'range'" = !missing(samples), From 59be14280dd79774984b3c8bb70f083e0688e5d4 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 14:45:35 +0100 Subject: [PATCH 12/16] correct logical expr in if in extract_param --- R/extract_param.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index 3819d8479..d28c997f9 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -101,7 +101,7 @@ extract_param <- function(type = c("percentiles", "range"), } if (identical(type, "range")) { - if (values[2] < values[1] && values[1] < values[3]) { + if (!(values[2] < values[1] && values[1] < values[3])) { stop("Distribution is ", distribution, " Values are ", values, " Type is ", type) } From 1c6345e231762f6db7420972112122dcb860eff0 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 15:29:00 +0100 Subject: [PATCH 13/16] added while loop to extract-bias vignette to prevent incoherent values --- R/extract_param.R | 14 +--- vignettes/extract-bias.Rmd | 155 ++++++++++++++++++++----------------- 2 files changed, 87 insertions(+), 82 deletions(-) diff --git a/R/extract_param.R b/R/extract_param.R index d28c997f9..965cb0fd8 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -81,12 +81,7 @@ extract_param <- function(type = c("percentiles", "range"), if (distribution == "norm") { checkmate::assert_numeric(values) } else { - - # if (any(values <= 1e-10)) { - # stop("Distribution is ", distribution, " Values are ", values, " Samples are ", samples) - # } - - checkmate::assert_numeric(values, lower = 0) + checkmate::assert_numeric(values, lower = 1e-10) } if (type == "percentiles") { @@ -100,11 +95,6 @@ extract_param <- function(type = c("percentiles", "range"), checkmate::assert_numeric(percentiles, lower = 0, upper = 1, len = 2) } if (identical(type, "range")) { - - if (!(values[2] < values[1] && values[1] < values[3])) { - stop("Distribution is ", distribution, " Values are ", values, " Type is ", type) - } - stopifnot( "samples need to be given for type = 'range'" = !missing(samples), @@ -231,7 +221,7 @@ extract_param <- function(type = c("percentiles", "range"), } if (distribution == "weibull") { names(param) <- c("shape", "scale") - lower <- c(1e-5, 1e-5) + lower <- c(1e-10, 1e-10) } if (distribution == "norm") { names(param) <- c("mean", "sd") diff --git a/vignettes/extract-bias.Rmd b/vignettes/extract-bias.Rmd index cbe422314..2174c333f 100644 --- a/vignettes/extract-bias.Rmd +++ b/vignettes/extract-bias.Rmd @@ -206,46 +206,54 @@ estim_params <- vector("list", nrow(parameters_range)) for (params_idx in seq_len(nrow(parameters_range))) { dist <- as.character(parameters_range[params_idx, "dist"]) n_samples <- parameters_range[params_idx, "n_samples"] - if (dist == "lnorm") { - true_median <- do.call( - paste0("q", dist), - list( - p = 0.5, - meanlog = parameters_range[params_idx, "param_1"], - sdlog = parameters_range[params_idx, "param_2"] + + # while loop to ensure values are min < median < max + resample_values <- TRUE + while (resample_values) { + if (dist == "lnorm") { + true_median <- do.call( + paste0("q", dist), + list( + p = 0.5, + meanlog = parameters_range[params_idx, "param_1"], + sdlog = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- do.call( - paste0("r", dist), - list( - n = n_samples, - meanlog = parameters_range[params_idx, "param_1"], - sdlog = parameters_range[params_idx, "param_2"] + true_range <- do.call( + paste0("r", dist), + list( + n = n_samples, + meanlog = parameters_range[params_idx, "param_1"], + sdlog = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- c(min(true_range), max(true_range)) - } else { - true_median <- do.call( - paste0("q", dist), - list( - p = 0.5, - shape = parameters_range[params_idx, "param_1"], - scale = parameters_range[params_idx, "param_2"] + true_range <- c(min(true_range), max(true_range)) + } else { + true_median <- do.call( + paste0("q", dist), + list( + p = 0.5, + shape = parameters_range[params_idx, "param_1"], + scale = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- do.call( - paste0("r", dist), - list( - n = n_samples, - shape = parameters_range[params_idx, "param_1"], - scale = parameters_range[params_idx, "param_2"] + true_range <- do.call( + paste0("r", dist), + list( + n = n_samples, + shape = parameters_range[params_idx, "param_1"], + scale = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- c(min(true_range), max(true_range)) + true_range <- c(min(true_range), max(true_range)) + } + + true_values <- c(true_median, true_range) + if (true_values[2] < true_values[1] && true_values[1] < true_values[3]) { + resample_values <- FALSE + } } - - true_values <- c(true_median, true_range) - + # message about stochastic optimisation suppressed estim_params[[params_idx]] <- suppressMessages( expr = extract_param( @@ -392,46 +400,53 @@ for (params_idx in seq_len(nrow(parameters_range))) { dist <- as.character(parameters_range[params_idx, "dist"]) n_samples <- parameters_range[params_idx, "n_samples"] - if (dist == "lnorm") { - true_median <- do.call( - paste0("q", dist), - list( - p = 0.5, - meanlog = parameters_range[params_idx, "param_1"], - sdlog = parameters_range[params_idx, "param_2"] + # while loop to ensure values are min < median < max + resample_values <- TRUE + while (resample_values) { + if (dist == "lnorm") { + true_median <- do.call( + paste0("q", dist), + list( + p = 0.5, + meanlog = parameters_range[params_idx, "param_1"], + sdlog = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- do.call( - paste0("r", dist), - list( - n = n_samples, - meanlog = parameters_range[params_idx, "param_1"], - sdlog = parameters_range[params_idx, "param_2"] + true_range <- do.call( + paste0("r", dist), + list( + n = n_samples, + meanlog = parameters_range[params_idx, "param_1"], + sdlog = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- c(min(true_range), max(true_range)) - } else { - true_median <- do.call( - paste0("q", dist), - list( - p = 0.5, - shape = parameters_range[params_idx, "param_1"], - scale = parameters_range[params_idx, "param_2"] + true_range <- c(min(true_range), max(true_range)) + } else { + true_median <- do.call( + paste0("q", dist), + list( + p = 0.5, + shape = parameters_range[params_idx, "param_1"], + scale = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- do.call( - paste0("r", dist), - list( - n = n_samples, - shape = parameters_range[params_idx, "param_1"], - scale = parameters_range[params_idx, "param_2"] + true_range <- do.call( + paste0("r", dist), + list( + n = n_samples, + shape = parameters_range[params_idx, "param_1"], + scale = parameters_range[params_idx, "param_2"] + ) ) - ) - true_range <- c(min(true_range), max(true_range)) + true_range <- c(min(true_range), max(true_range)) + } + + true_values <- c(true_median, true_range) + if (true_values[2] < true_values[1] && true_values[1] < true_values[3]) { + resample_values <- FALSE + } } - - true_values <- c(true_median, true_range) - + # message about stochastic optimisation suppressed estim <- suppressMessages( replicate( From 8983aab0902d73fe569391f23efc9e2ca7cc2045 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 15:33:00 +0100 Subject: [PATCH 14/16] increase weibull lower bound --- R/extract_param.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index 965cb0fd8..deb9f36a4 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -221,7 +221,7 @@ extract_param <- function(type = c("percentiles", "range"), } if (distribution == "weibull") { names(param) <- c("shape", "scale") - lower <- c(1e-10, 1e-10) + lower <- c(1e-5, 1e-5) } if (distribution == "norm") { names(param) <- c("mean", "sd") From cbd2ec80ecfd7ef2773c36374c4dc383c5f30ce1 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Wed, 1 May 2024 15:37:35 +0100 Subject: [PATCH 15/16] revert to lower value bound at 0 --- R/extract_param.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract_param.R b/R/extract_param.R index deb9f36a4..d8a359600 100644 --- a/R/extract_param.R +++ b/R/extract_param.R @@ -81,7 +81,7 @@ extract_param <- function(type = c("percentiles", "range"), if (distribution == "norm") { checkmate::assert_numeric(values) } else { - checkmate::assert_numeric(values, lower = 1e-10) + checkmate::assert_numeric(values, lower = 0) } if (type == "percentiles") { From d13640d785d28149bd900f5036323ab80b07b738 Mon Sep 17 00:00:00 2001 From: Hugo Gruson <10783929+Bisaloo@users.noreply.github.com> Date: Wed, 1 May 2024 17:14:10 +0200 Subject: [PATCH 16/16] Remove fail fast So we can see what happens in other runners --- .github/workflows/R-CMD-check.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 62b0eff0b..251a9d100 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -85,10 +85,3 @@ jobs: with: upload-snapshots: true error-on: '"note"' - - # fail-fast but only if rcmdcheck step fails - - name: Manual fail-fast - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - if: always() && steps.rcmdcheck.outcome == 'failure' - run: gh run cancel ${{ github.run_id }}