metrumresearchgroup / bbr

R interface for model and project management
https://metrumresearchgroup.github.io/bbr/
Other
23 stars 2 forks source link

`open_output_file` helper #569

Open seth127 opened 1 year ago

seth127 commented 1 year ago

Consider adding open_ helpers for output files, primarily for monitoring runs in progress. Some code ideas below for implementation ideas.

One proposal is to have the second arg match on any substring in the file name (instead of .extension) so that users can easily open other files like OFV.TXT without having to remember the full name of the file.

# the ... will let us potentially through other params for other model types, 
# i.e. a .chain param for NONMEM-Bayes
open_output_file <- function(.mod, .extension, ...){
  UseMethod("open_output_file")
}

open_output_file_impl <- function(.dir, .extension){
  ## grab file with .extension
  tmpfile <- list.files(.dir, pattern=regex(paste0(.extension, "$")))

  ## return NULL if file doesn't exist
  if(length(tmpfile) == 0){
    warning("File does not exist")
    return(NULL)
  }
  if(length(tmpfile) > 1) { 
    # probably error and return the paths?
    # not likely to happen, but we should catch it
  }

  ## open file
  file.edit(file.path(.dir,tmpfile))
}

# we want this to default to OUTPUT, because that's the primary log file
# but also be able to be used as it is in `open_lst_file()` below
open_output_file.bbi_nonmem_model <- function(.mod, .extension = "OUTPUT", ...){
  if (.extension == "OUTPUT" && (check_nonmem_finished(.mod)) {
    message("Model finished running. No OUTPUT file present.")
    return(invisible(NULL))
  }
  open_output_file_impl(.dir = get_output_dir(.mod), .extension = .extension)
}

# simple convenience helper for basic NONMEM
open_lst_file <- function(.mod){
  open_output_file(.mod, .extension="lst")
}

# idea for NONMEM-Bayes implementation (would go in bbr.bayes)
open_output_file.bbi_nmbayes_model <- function(.mod, .extension = "OUTPUT", .chain, ...){
  # check_nonmem_finished() isn't implemented yet for nmbayes but it will be at some point
  if (.extension == "OUTPUT" && (check_nonmem_finished(.mod)) {
    message("Model finished running. No OUTPUT file present.")
    return(invisible(NULL))
  }

  # I think there's some helper like this already, but I can't remember the mechanics
  # either way, this builds the path to the output dir of the specified chain
  chain_dir <- get_output_dir(.mod, .chain) 

  open_output_file_impl(.dir = chain_dir, .extension = .extension)
}
kyleam commented 1 year ago

primarily for monitoring runs in progress

Given automatic refreshing in RStudio, I'm guessing your feeling is that RStudio users will find these more useful than tail_output and tail_lst?

seth127 commented 1 year ago

Given automatic refreshing in RStudio, I'm guessing your feeling is that RStudio users will find these more useful than tail_output and tail_lst?

Exactly. I talked to a few folks at ACoP who were not finding the tail_ helpers that... helpful. And one of them gave me this idea, which I think is a good one.

seth127 commented 1 year ago

@kyleam I'm considering trying to sneak in a small bbr release before the next MPN, basically with what I put on https://github.com/metrumresearchgroup/bbr/milestone/28 , and I'm interested in your thoughts. I'm commenting on this issue because I think it is the one that has the potential to get complicated (though I hope not).

My primary motivation here is that we are planning to update the Expo repo to the newest MPN, after the next snapshot, and I would like to be able mention the new open_* helpers (this one and #570 that already merged).

Any thoughts on this are welcome, but the things on my mind currently are:

Relevant: #570 is the only that's merged to main since 1.5.0.

kyleam commented 1 year ago

Are there any complicating factors in this issue that I'm not thinking of?

Nothing's popping out to me. The main thing is that it's probably worth reviewing R/read-output.R to try to avoid having duplicate internal logic.

Should this be 1.6.0 or 1.5.1?

I'd say 1.6.0 because it'd have new features.

Is there anything else little that we should knock out and include?

I can't think of anything that need to be prioritized for this release.

barrettk commented 1 year ago

@seth127 Just making a note of it since I didnt see an issue for it, but we may want to include the model_summary() bug for when no directory exists. I know you said you didnt run into this issue at one point on your machine, but may be worth testing for the next release

seth127 commented 1 year ago

We're going to hold off on this release for now (probably another week or two, at least) because of other priorities. On this...

the model_summary() bug for when no directory exists.

As you note, I wasn't able to reproduce it with the newest bbr, but I may have been doing something different. See you can reproduce it outside of the diagnostics apps context and, if you can, go ahead and make an issue.

FWIW, I tried to reproduce the error with this and couldn't:

devtools::load_all()

.md <- system.file("model", "nonmem", "basic", package = "bbr")

mod1 <- read_model(file.path(.md, 1))
mod2 <- copy_model_from(mod1)

# these all work for me
summary_log(.md)
run_log(.md) %>% add_summary()
model_summaries(list(mod1, mod2))

# this fails, but that's fine
model_summary(mod2)
# Error in get_output_dir_nonmem(.bbi_object, .check_exists) : ...

delete_models(mod2, .tags = NULL)
kyleam commented 1 year ago

may want to include the model_summary() bug for when no directory exists

@seth127 @barrettk Should be resolved by gh-573 (see first two commits).