Conversation
|
@roninsightrx @jasmineirx Added some functionality to aim/iterate/report t_gt_mic metrics common to beta-lactam therapeutics. Would appreciate suggestions / feedback as to how this should be better formulated. Not quite there yet but steps in the right direction. |
JordanBrooks33
changed the title
jasmineirx
left a comment
jasmineirx
left a comment
There was a problem hiding this comment.
Great to have this functionality in place. I think we need a few changes to ensure this update works generically for a broad range of inputs.
| max = upperbound, | ||
| scheme = scheme | ||
| ) | ||
| if(target_design$type %in% target_types_time) { |
There was a problem hiding this comment.
out of curiosity, why put the new logic here, after we create the target_design object, rather than above, where we do other target parsing?
| "CONC" | ||
| ) | ||
| target_design$type <- "cmin" | ||
| target_design$value <- min(target_design$range)/100 * tail(covariates$MIC$value, 1) |
There was a problem hiding this comment.
i believe this is pulling covariates from the global environment, which is somewhat error prone, and should be avoided. This is particularly true since target design is usually defined up front while covariates may change patient-to-patient.
| if(nrow(target_design$scheme) > 1) { | ||
| cli::cli_abort("Please provide only single non-time-varying target designs to `dose_grid_search()`") | ||
| } | ||
|
|
There was a problem hiding this comment.
could you provide some insight in the PR description regarding some design choices, e.g., why this had to be removed from this function and added to create_target_design?
| if (target_design$type %in% target_types_time) { | ||
| grid <- seq(1, 10000, by = 100) | ||
| } |
There was a problem hiding this comment.
| if (target_design$type %in% target_types_time) { | |
| grid <- seq(1, 10000, by = 100) | |
| } | |
| # time-based targets are highly non-linear and so a fine-grain grid helps to | |
| # better identify the right dose | |
| if (target_design$type %in% target_types_time) { | |
| grid <- seq(min(grid), max(grid), by = 100) | |
| } |
I think a code comment would be useful here to clarify why time-type targets require a 100-mg increment grain. this is also more hard-coded than we usually do, and could fail surprisingly for a drug where dose sizes are typically expressed in grams. for example, methotrexate is usually around 12000 mg (although I recognize that we dont typically target time above a threshold for that drug...). is there a reason we have to hard-code this at all when the grid argument is accessible for the user to specify?
| regimen = reg | ||
| ) | ||
| if(length(t_obs) > 1 && !target_design$type %in% c("auc", target_types_time)) { | ||
| if(length(t_obs) > 1 && !target_design$type %in% c("auc", target_types_auc, target_types_time)) { |
There was a problem hiding this comment.
| if(length(t_obs) > 1 && !target_design$type %in% c("auc", target_types_auc, target_types_time)) { | |
| if(length(t_obs) > 1 && !target_design$type %in% c(target_types_auc, target_types_time)) { |
"auc" is in target_types_auc so we don't need to include it twice
| get_quantity_from_variable( | ||
| var = target_design$type, | ||
| sim = tmp, | ||
| md = md, | ||
| times = t_obs, | ||
| comp = "obs" | ||
| ), | ||
| 1 | ||
| ) |
There was a problem hiding this comment.
can you explain why this change was necessary? It looks like we are going from a more flexible/generic solution (get_quantity_from_variable handles other variables) to a more hardcoded solution (TGTMIC-type targets only).
| tgt_use <- case_when(target_design$type == "t_gt_4mic_free" ~ "FTGT4MIC", | ||
| target_design$type == "t_gt_mic_free" ~ "FTGTMIC", | ||
| target_design$type == "t_gt_4mic" ~ "TGT4MIC", | ||
| target_design$type == "t_gt_mic" ~ "TGTMIC") | ||
| 100*diff(sim_output[sim_output$comp == "obs",][[tgt_use]])/regimen$interval |
There was a problem hiding this comment.
this logic is similar to what is being used in dose_grid_search. Maybe it makes sense to make a little helper function so that if we add additional time/MIC targets we only have to modify one place.
| return(target_design) | ||
| } | ||
| if(idx > nrow(target_design$scheme)) { | ||
| if(idx > nrow(target_design$scheme) | idx > length(target_design$value) ) { |
There was a problem hiding this comment.
| if(idx > nrow(target_design$scheme) | idx > length(target_design$value) ) { | |
| if (idx > nrow(target_design$scheme) || idx > length(target_design$value) ) { |
for if conditionals, we should always use scalar operators and not vector operators.
3 participants
like "time % of free conc above MIC".