vpcPlot() discards a supplied nlmixr2vpcSim object and re-simulates at n = 300

[mattfidler]

Summary

Passing a pre-computed nlmixr2vpcSim object to vpcPlot() (or vpcCens() / vpcPlotTad() / vpcCensTad()) does not use it. The object is discarded and a fresh simulation is run with the default n = 300, so the work is thrown away, n is silently ignored, and any seed/settings baked into the supplied simulation are lost.

Reproducer

s5 <- nlmixr2est::vpcSim(fit, n = 5, pred = TRUE, seed = 7)
nrow(s5)                     #> 660
length(unique(s5$sim.id))    #> 5

db <- vpcCens(s5, cens = TRUE, vpcdb = TRUE)
nrow(db$sim)                 #> 39600   <-- re-simulated at n = 300
length(unique(db$sim$sim))   #> 300     <-- not 5

# uncensored path too
nrow(vpcPlot(s5, vpcdb = TRUE)$sim)   #> 39600  vs supplied 660

Root cause

R/vpcPlot.R. The guard reassigns fit before the check that depends on it:

if (inherits(fit, "nlmixr2vpcSim")) {
  .sim <- fit
  .fit <- attr(class(.sim), "fit")
  ...
  fit <- .fit            # fit is now the plain fit object
}

...

# Simulate with VPC
if (!inherits(fit, "nlmixr2vpcSim")) {   # always TRUE by now
  .sim <- nlmixr2est::vpcSim(fit, ..., keep = stratify, n = n, pred = pred_corr, seed = seed)
}

Because fit has been replaced with the underlying fit, the second condition is always true and .sim is always overwritten.

Suggested fix

Record whether a simulation was supplied, e.g.

.hasSim <- inherits(fit, "nlmixr2vpcSim")
if (.hasSim) { ... fit <- .fit }
...
if (!.hasSim) {
  .sim <- nlmixr2est::vpcSim(...)
}

Worth checking at the same time whether a supplied simulation carries the keep = stratify columns that vpcSimExpand() expects, since that path has never actually been exercised.

Impact

Affects both the censored and uncensored paths. Mostly wasted computation and a silently ignored n, but it also means the "pass a pre-computed vpcSim object" workflow has effectively never been tested - tests that pass such an object are really just re-testing the fit path. I noted this on the tests added in #54 rather than leave them claiming coverage they do not have.

Context

Pre-existing on main; found while reviewing #54.