-
Notifications
You must be signed in to change notification settings - Fork 2
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
Function to run NMTRAN
on a model
#652
Conversation
@seth127 when you get a second, let me know what you think of the compare_nmtran function. I assume we will ditch it, but i'd like to keep that function handy during some development tasks that involve touching control stream files. I suppose we could export it, though I dont think that's too in-line with the original request. It is comparable to the bash script @kyleam was running though. Edit: Since I removed the compare_nmtran#' Compare different NONMEM control stream configurations.
#'
#' Runs `run_nmtran()` on two models and compares the output, denoting whether
#' they evaluate to the same model via `NMTRAN`.
#'
#' @details
#' Say you wanted to test whether diagonal matrices could specify standard
#' deviation for one value, and variance for another
#'
#' The **reference model** would have this block:
#' ```r
#' $OMEGA
#' 0.05 STANDARD ; iiv CL
#' 0.2 ; iiv V2
#' ```
#'
#' The **new model** would have this block:
#' ```r
#' $OMEGA
#' 0.05 STANDARD ; iiv CL
#' 0.2 VAR ; iiv V2
#' ```
#'
#' Comparing the two (see below), we find no differences. This means that adding
#' `VAR` to the second ETA value had no impact, and the two models would evaluate
#' the same.
#' ```r
#' > compare_nmtran(MOD1, MOD_COMPARE)
#' Running NMTRAN with NONMEM version `nm75`
#'
#' No differences found
#' character(0)
#' ```
#'
#' @examples
#' \dontrun{
#' # Starting model - set a reference
#' open_model_file(MOD1)
#'
#' # Make new model
#' MOD_COMPARE <- copy_model_from(MOD1)
#'
#' # Make a change
#' open_model_file(MOD_COMPARE)
#'
#' # Compare NMTRAN evaluation
#' compare_nmtran(MOD1, MOD_COMPARE)
#'
#' # delete new model at the end
#' delete_models(MOD_COMPARE, .tags = NULL, .force = TRUE)
#' }
#'
#' @keywords internal
compare_nmtran <- function(
.mod,
.mod_compare,
.config_path = NULL,
nmtran_exe = NULL
){
# Set NMTRAN executable
nmtran_exe <- locate_nmtran(.mod, .config_path, nmtran_exe)
nmtran_exe2 <- locate_nmtran(.mod_compare, .config_path, nmtran_exe)
# This would only happen when comparing two models in different working
# directories, where the `bbi.yaml` defaults differ.
if(nmtran_exe != nmtran_exe2){
rlang::warn(
c(
"!" = "Found two separate NMTRAN executables:",
" " = paste("-", nmtran_exe),
" " = paste("-", nmtran_exe2),
"i" = "Defaulting to the first one"
)
)
}
# This function is used to remove problem statement differences introduced
# via `copy_model_from()`
empty_prob_statement <- function(.mod){
mod_new <- copy_model_from(.mod, paste0(get_model_id(.mod), "_no_prob"))
mod_path <- get_model_path(mod_new)
ctl <- nmrec::read_ctl(mod_path)
prob_rec <- nmrec::select_records(ctl, "prob")[[1]]
prob_rec$parse()
# Overwrite 'text' option
prob_rec$values <- purrr::map(prob_rec$values, function(prob_rec){
if(inherits(prob_rec, "nmrec_option_pos") && prob_rec$name == "text"){
prob_rec$value <- ""
}
prob_rec
})
# Write out modified ctl
nmrec::write_ctl(ctl, mod_path)
return(mod_new)
}
# Remove problem statements from both models to ensure a fair comparison
# If update_model_id was called, this would also change the evaluation,
# though we can't really prevent that.
mod_no_prob <- empty_prob_statement(.mod)
compare_no_prob <- empty_prob_statement(.mod_compare)
on.exit(
delete_models(
list(mod_no_prob, compare_no_prob), .tags = NULL, .force = TRUE
) %>% suppressMessages(),
add = TRUE
)
# Run NMTRAN on each model
nmtran_mod <- run_nmtran(
mod_no_prob, nmtran_exe = nmtran_exe,
delete_on_exit = FALSE
)
nmtran_compare <- run_nmtran(
compare_no_prob, nmtran_exe = nmtran_exe,
delete_on_exit = FALSE
)
# Force delete folders at the end
on.exit(unlink(nmtran_mod$run_dir, recursive = TRUE, force = TRUE), add = TRUE)
on.exit(unlink(nmtran_compare$run_dir, recursive = TRUE, force = TRUE), add = TRUE)
# Compare FCON files
nmtran_mod_fcon <- file.path(nmtran_mod$run_dir, "FCON")
nmtran_compare_fcon <- file.path(nmtran_compare$run_dir, "FCON")
cmd <- paste("cmp", nmtran_mod_fcon, nmtran_compare_fcon)
# Compare FCON files from each NMTRAN run
.p <- processx::process$new(
command = "cmp", args = c(nmtran_mod_fcon, nmtran_compare_fcon),
stdout = "|", stderr = "|"
)
# Format output
output <- .p$read_all_output_lines()
if(length(output) == 0){
cat_line("No differences found", col = "green")
}else{
cat_line("Models are not equivalent", col = "red")
output <- gsub(paste0(nmtran_mod_fcon, "|", nmtran_compare_fcon), "", output) %>%
stringr::str_trim()
}
return(output)
} |
- Searches for NMTRAN executable using `bbi.yaml`, though allows for passing the path directly. - also includes `compare_nmtran` developer tool, which can be useful for comparing different NONMEM control stream configurations (see details of function for an example)
- dont need to evaluate it within a different environment
- this likely makes windows unsupported
- did a lot of testing with processx, but wasnt able to get the command to work due to the function's insistence on quoting the arguements.
- use processx instead of `system()` - add print options for NMTRAN run - improved `compare_nmtran` function
- utilize get_data_path refactor
This is looking good to me, but I'll leave the final review for @andersone1 |
@kyleam could you take a look at how we're building the path to the NMTRAN executable and make any comment on whether we think this will work on Windows. And if not... do we know a better pattern for that? Related, we execute it with |
Looking just at those linked lines, nothing jumps out at me that will be an issue. That's assuming all the processing outside those lines is correct on Windows, but the best option would of course just be to try it out. Before this is merged, I can test the command on our Windows instance for bbr/bbi testing. Or perhaps better: I can get @andersone1 and @barrettk set up to access. Let me know what you all prefer.
Right. processx in general supports Windows. |
I'll pull my comment from #650 (comment) for consideration:
|
@barrettk @andersone1 see both comments from @kyleam above. Could the two of you coordinate to do the following:
Thanks all. I'm looking forward to getting this feature out there. |
- a better version will be available after rebasing (I had redone this for the bootstrap PR), and deleting it ahead of time helps avoid some conflicts with the rebase
Addressing the conflicts via a rebase ended up not being worth the trouble, so opened a new PR here |
Args
Example
closes #650