-
Notifications
You must be signed in to change notification settings - Fork 6
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
Issue 3: Not using DT. #4
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,8 +3,7 @@ | |||||
sample_model <- function(model, data, scenario = data.table::data.table(id = 1), | ||||||
diagnostics = TRUE, ...) { | ||||||
|
||||||
out <- scenario |> | ||||||
copy() | ||||||
out <- data.table::copy(scenario) | ||||||
|
||||||
# Setup failure tolerant model fitting | ||||||
fit_model <- function(model, data, ...) { | ||||||
|
@@ -16,12 +15,10 @@ | |||||
fit <- safe_fit_model(model, data, ...) | ||||||
|
||||||
if (!is.null(fit$error)) { | ||||||
out <- out |> | ||||||
DT(, error := list(fit$error[[1]])) | ||||||
out[, error := list(fit$error[[1]])] | ||||||
diagnostics <- FALSE | ||||||
}else { | ||||||
out <- out |> | ||||||
DT(, fit := list(fit$result)) | ||||||
out[, fit := list(fit$result)] | ||||||
fit <- fit$result | ||||||
} | ||||||
|
||||||
|
@@ -40,12 +37,12 @@ | |||||
per_divergent_transitions = sum(diag$divergent__) / nrow(diag), | ||||||
max_treedepth = max(diag$treedepth__) | ||||||
) | ||||||
diagnostics[, no_at_max_treedepth := sum(diag$treedepth__ == max_treedepth)] | ||||||
Check warning on line 40 in R/fitting-and-postprocessing.R
|
||||||
diagnostics[, per_at_max_treedepth := no_at_max_treedepth / nrow(diag)] | ||||||
Check warning on line 41 in R/fitting-and-postprocessing.R
|
||||||
out <- cbind(out, diagnostics) | ||||||
|
||||||
timing <- round(fit$time()$total, 1) | ||||||
out[, run_time := timing] | ||||||
} | ||||||
return(out[]) | ||||||
} | ||||||
|
@@ -57,8 +54,7 @@ | |||||
diagnostics = TRUE, ... | ||||||
) { | ||||||
|
||||||
out <- scenario |> | ||||||
copy() | ||||||
out <- data.table::copy(scenario) | ||||||
|
||||||
# Setup failure tolerant model fitting | ||||||
fit_model <- function(model, data, ...) { | ||||||
|
@@ -72,12 +68,10 @@ | |||||
fit <- safe_fit_model(model, data, ...) | ||||||
|
||||||
if (!is.null(fit$error)) { | ||||||
out <- out |> | ||||||
DT(, error := list(fit$error[[1]])) | ||||||
out[, error := list(fit$error[[1]])] | ||||||
diagnostics <- FALSE | ||||||
}else { | ||||||
out <- out |> | ||||||
DT(, fit := list(fit$result)) | ||||||
out[, fit := list(fit$result)] | ||||||
fit <- fit$result | ||||||
} | ||||||
|
||||||
|
@@ -110,8 +104,8 @@ | |||||
per_divergent_transitions = sum(diag$divergent__) / nrow(diag), | ||||||
max_treedepth = max(diag$treedepth__) | ||||||
) | ||||||
diagnostics[, no_at_max_treedepth := sum(diag$treedepth__ == max_treedepth)] | ||||||
Check warning on line 107 in R/fitting-and-postprocessing.R
|
||||||
diagnostics[, per_at_max_treedepth := no_at_max_treedepth / nrow(diag)] | ||||||
out <- cbind(out, diagnostics) | ||||||
|
||||||
timing <- round(max(fit$metadata()$time$total), 1) | ||||||
|
@@ -123,11 +117,12 @@ | |||||
#' Add natural scale summary parameters for a lognormal distribution | ||||||
#' @export | ||||||
add_natural_scale_mean_sd <- function(dt) { | ||||||
nat_dt <- dt |> | ||||||
data.table::DT(, mean := exp(meanlog + sdlog ^ 2 / 2)) |> | ||||||
data.table::DT(, | ||||||
sd := exp(meanlog + (1 / 2) * sdlog ^ 2) * sqrt(exp(sdlog ^ 2) - 1) | ||||||
) | ||||||
nat_dt <- data.table::copy(dt) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the original (i.e. above) we aren't making a copy but I think its fine either way (though add might imply no copy?) |
||||||
|
||||||
nat_dt <- nat_dt[,mean := exp(meanlog + sdlog ^ 2 / 2)] | ||||||
|
||||||
nat_dt <- nat_dt[,sd := exp(meanlog + (1 / 2) * sdlog ^ 2) * sqrt(exp(sdlog ^ 2) - 1)] | ||||||
|
||||||
return(nat_dt[]) | ||||||
} | ||||||
|
||||||
|
@@ -186,8 +181,7 @@ | |||||
) | ||||||
} | ||||||
|
||||||
draws <- draws |> | ||||||
data.table::setDT() | ||||||
draws <- data.table::setDT(draws) | ||||||
|
||||||
data.table::setnames( | ||||||
draws, c("refp_mean_int[1]", "refp_sd_int[1]"), c("meanlog", "sdlog"), | ||||||
|
@@ -207,10 +201,11 @@ | |||||
#' Primary event bias correction | ||||||
#' @export | ||||||
primary_censoring_bias_correction <- function(draws) { | ||||||
draws <- data.table::copy(draws) |> | ||||||
DT(, mean := mean - runif(.N, min = 0, max = 1)) |> | ||||||
DT(, meanlog := log(mean^2 / sqrt(sd^2 + mean^2))) |> | ||||||
DT(, sdlog := sqrt(log(1 + (sd^2 / mean^2)))) | ||||||
draws <- data.table::copy(draws) | ||||||
draws[, mean := mean - runif(.N, min = 0, max = 1)] | ||||||
draws[, meanlog := log(mean^2 / sqrt(sd^2 + mean^2))] | ||||||
draw[, sdlog := sqrt(log(1 + (sd^2 / mean^2)))] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
return(draws[]) | ||||||
} | ||||||
|
||||||
|
@@ -234,7 +229,8 @@ | |||||
by = by | ||||||
) | ||||||
|
||||||
draws <- draws[, rel_value := value / true_value] | ||||||
draws[, rel_value := value / true_value] | ||||||
|
||||||
return(draws[]) | ||||||
} | ||||||
|
||||||
|
@@ -289,9 +285,11 @@ | |||||
if (missing(variable)) { | ||||||
stop("variable must be specified") | ||||||
} | ||||||
summarised_draws <- draws |> | ||||||
copy() |> | ||||||
DT(, value := variable, env = list(variable = variable)) |> | ||||||
summarise_draws(sf = sf, by = by) | ||||||
summarised_draws <- data.table::copy(draws) | ||||||
|
||||||
summarised_draws[, value := variable, env = list(variable = variable)] | ||||||
|
||||||
summarised_draws <- summarise_draws(summarised_draws, sf = sf, by = by) | ||||||
|
||||||
return(summarised_draws[]) | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an annoying side effect of linting. Need to add error as a global var in utils to get rid of this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we can update the linter to ignore global var checks (but then Rmd check will still flag this anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively the env features we use elsewhere may help here?