From de3675e5950ca02f2c3fd37e63272ef83124abb5 Mon Sep 17 00:00:00 2001
From: Bill Denney <wdenney@humanpredictions.com>
Date: Tue, 19 Mar 2024 11:47:04 -0400
Subject: [PATCH 1/5] Add failing test (it will likely be modified later)

---
 tests/testthat/test-tar_nlmixr_multimodel.R | 47 +++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tests/testthat/test-tar_nlmixr_multimodel.R b/tests/testthat/test-tar_nlmixr_multimodel.R
index 184beff..36812d0 100644
--- a/tests/testthat/test-tar_nlmixr_multimodel.R
+++ b/tests/testthat/test-tar_nlmixr_multimodel.R
@@ -169,3 +169,50 @@ targets::tar_test("tar_nlmixr_multimodel works with initial condition setting `c
   # to the `central(0) <- 0` line
   expect_type(targets::tar_outdated(callr_function = NULL), "character")
 })
+
+test_that("tar_nlmixr_multimodel works for within-list model piping (#19)", {
+  pheno <- function() {
+    ini({
+      lcl <- log(0.008); label("Typical value of clearance")
+      lvc <-  log(0.6); label("Typical value of volume of distribution")
+      etalcl + etalvc ~ c(1,
+                          0.01, 1)
+      cpaddSd <- 0.1; label("residual variability")
+    })
+    model({
+      cl <- exp(lcl + etalcl)
+      vc <- exp(lvc + etalvc)
+      kel <- cl/vc
+      d/dt(central) <- -kel*central
+      cp <- central/vc
+      cp ~ add(cpaddSd)
+    })
+  }
+
+  target_list <-
+    tar_nlmixr_multimodel(
+      name = foo, data = nlmixr2data::pheno_sd, est = "saem",
+      "my first model" = pheno,
+      "my second model" = foo[["my first model"]] |> rxode2::ini(lcl = log(0.01))
+    )
+  expect_true(inherits(target_list, "list"))
+  # One for each model and then one for combining everything
+  expect_length(target_list, 3)
+  # Data and object simplification, then the fitting
+  expect_length(target_list[[1]], 4)
+  # Data and object simplification, then the fitting
+  expect_length(target_list[[2]], 4)
+  # Combine the fit models as a single step
+  expect_s3_class(target_list[[3]], "tar_stem")
+  expect_equal(target_list[[3]]$settings$name, "foo")
+
+  # Verify the expression for collation is generated correctly
+  collating_call <- target_list[[3]]$command$expr[[1]]
+  expect_true(grepl(x = as.character(collating_call[[2]]), pattern = "^foo_[0-9a-f]{8}$"))
+  expect_true(grepl(x = as.character(collating_call[[3]]), pattern = "^foo_[0-9a-f]{8}$"))
+  expect_equal(names(collating_call), c("", "my first model", "my second model"))
+
+  # Verify the targets created are the ones being collated
+  expect_equal(collating_call[[2]], as.name(target_list[[1]][[4]]$settings$name))
+  expect_equal(collating_call[[3]], as.name(target_list[[2]][[4]]$settings$name))
+})

From d1f1ddc0ed10b21694cd49600e2a2f92684ede6c Mon Sep 17 00:00:00 2001
From: Bill Denney <wdenney@humanpredictions.com>
Date: Wed, 20 Mar 2024 10:19:37 -0400
Subject: [PATCH 2/5] Allow models to depend on each other within a set of
 target models for piping

---
 R/tar_nlmixr.R                              | 110 +++++++++---------
 R/tar_nlmixr_multimodel.R                   | 117 +++++++++++++++++++-
 tests/testthat/test-tar_nlmixr_multimodel.R |  58 +++++++++-
 3 files changed, 225 insertions(+), 60 deletions(-)

diff --git a/R/tar_nlmixr.R b/R/tar_nlmixr.R
index e243b05..e5e15df 100644
--- a/R/tar_nlmixr.R
+++ b/R/tar_nlmixr.R
@@ -82,60 +82,64 @@ tar_nlmixr_raw <- function(name, object, data, est, control, table, object_simpl
   # Make models with initial conditions set work within `targets` (see #15)
   set_env_object_noinitial(object = object, env = env)
   list(
-    targets::tar_target_raw(
-      name = object_simple_name,
-      command =
-        substitute(
-          nlmixr_object_simplify(object = object),
-          list(object = object)
-        ),
-      packages = c("nlmixr2targets", "nlmixr2est")
-    ),
-    targets::tar_target_raw(
-      name = data_simple_name,
-      command =
-        substitute(
-          nlmixr_data_simplify(object = object_simple, data = data, table = table),
-          list(
-            object_simple = as.name(object_simple_name),
-            data = data,
-            table = table
-          )
-        ),
-      packages = "nlmixr2targets"
-    ),
-    targets::tar_target_raw(
-      name = fit_simple_name,
-      command =
-        substitute(
-          nlmixr2est::nlmixr(
-            object = object_simple_name,
-            data = data_simple_name,
-            est = est,
-            control = control
+    object_simple =
+      targets::tar_target_raw(
+        name = object_simple_name,
+        command =
+          substitute(
+            nlmixr_object_simplify(object = object),
+            list(object = object)
           ),
-          list(
-            object_simple_name = as.name(object_simple_name),
-            data_simple_name = as.name(data_simple_name),
-            est = est,
-            control = control,
-            table = table
-          )
-        ),
-      packages = "nlmixr2est"
-    ),
-    targets::tar_target_raw(
-      name = name,
-      command =
-        substitute(
-          assign_origData(fit = fit, data = data),
-          list(
-            fit = as.name(fit_simple_name),
-            data = data
-          )
-        ),
-      packages = "nlmixr2targets"
-    )
+        packages = c("nlmixr2targets", "nlmixr2est")
+      ),
+    data_simple =
+      targets::tar_target_raw(
+        name = data_simple_name,
+        command =
+          substitute(
+            nlmixr_data_simplify(object = object_simple, data = data, table = table),
+            list(
+              object_simple = as.name(object_simple_name),
+              data = data,
+              table = table
+            )
+          ),
+        packages = "nlmixr2targets"
+      ),
+    fit_simple =
+      targets::tar_target_raw(
+        name = fit_simple_name,
+        command =
+          substitute(
+            nlmixr2est::nlmixr(
+              object = object_simple_name,
+              data = data_simple_name,
+              est = est,
+              control = control
+            ),
+            list(
+              object_simple_name = as.name(object_simple_name),
+              data_simple_name = as.name(data_simple_name),
+              est = est,
+              control = control,
+              table = table
+            )
+          ),
+        packages = "nlmixr2est"
+      ),
+    fit =
+      targets::tar_target_raw(
+        name = name,
+        command =
+          substitute(
+            assign_origData(fit = fit, data = data),
+            list(
+              fit = as.name(fit_simple_name),
+              data = data
+            )
+          ),
+        packages = "nlmixr2targets"
+      )
   )
 }
 
diff --git a/R/tar_nlmixr_multimodel.R b/R/tar_nlmixr_multimodel.R
index 9227e21..84c3ccf 100644
--- a/R/tar_nlmixr_multimodel.R
+++ b/R/tar_nlmixr_multimodel.R
@@ -22,8 +22,15 @@ tar_nlmixr_multimodel <- function(name, ..., data, est, control = list(), table
   )
 }
 
+#' Generate nlmixr multimodel target set for all models in one call to
+#' `tar_nlmixr_multimodel()`
+#'
+#' @inheritParams tar_nlmixr_multimodel
+#' @inheritParams tar_nlmixr
+#' @keywords Internal
 tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_list, env) {
   checkmate::assert_named(model_list, type = "unique")
+
   ret_prep <-
     lapply(
       X = model_list,
@@ -35,6 +42,45 @@ tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_l
       table = table,
       env = env
     )
+  mask_self_referential <- tar_nlmixr_multimodel_has_self_reference(model_list = model_list, name = name)
+  while (any(mask_self_referential)) {
+    mask_self_referential_orig <- mask_self_referential
+    model_list_self_reference <- model_list[mask_self_referential]
+    name_map <-
+      setNames(
+        vapply(X = ret_prep, FUN = \(x) x$name, FUN.VALUE = ""),
+        # rxode2::.matchesLangTemplate() treats single vs double quotes in a
+        # call the same.
+        sprintf("%s[['%s']]", name, names(ret_prep))
+      )
+    model_list_fewer_self_ref <-
+      tar_nlmixr_multimodel_remove_self_reference(model_list = model_list[mask_self_referential], name_map = name_map)
+    # Replace self-referential models with possibly-not-self-referential models
+    model_list[names(model_list_fewer_self_ref)] <- model_list_fewer_self_ref
+    # Update the possibly-not-self-referential models
+    ret_prep[names(model_list_fewer_self_ref)] <-
+      lapply(
+        X = model_list[names(model_list_fewer_self_ref)],
+        FUN = tar_nlmixr_multimodel_single,
+        name = name,
+        data = data,
+        est = est,
+        control = control,
+        table = table,
+        env = env
+      )
+
+    mask_self_referential <- tar_nlmixr_multimodel_has_self_reference(model_list = model_list, name = name)
+    if (sum(mask_self_referential) >= sum(mask_self_referential_orig)) {
+      # The number of models which are self-referential should consistently
+      # decrease as dependencies are removed.  If this doesn't happen, then
+      # there is a circular reference somewhere.
+      stop(
+        "The following model(s) appear to have circular references to each other: ",
+        paste0('"', names(mask_self_referential)[mask_self_referential], '"', collapse = ", ")
+      )
+    }
+  }
   # Extract the targets to fit.  This will be a list of lists.  The inner list
   # will have the three targets for fitting the model, and the outer list will
   # be one element per model fit.
@@ -54,10 +100,75 @@ tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_l
   )
 }
 
+#' Does the model list refer to another model in the model list?
+#'
+#' @inheritParams tar_nlmixr_multimodel_parse
+#' @returns A logical vector the same length as `model_list` indicating if the
+#'   model is self-referential to another model in the list
+#' @keywords Internal
+tar_nlmixr_multimodel_has_self_reference <- function(model_list, name) {
+  sapply(X = model_list, FUN = tar_nlmixr_multimodel_has_self_reference_single, name = name)
+}
+#' @describeIn tar_nlmixr_multimodel_has_self_reference A helper function to
+#'   look at each call for each model separately
+tar_nlmixr_multimodel_has_self_reference_single <- function(model, name) {
+  if (rxode2::.matchesLangTemplate(model, str2lang(sprintf("%s[[.]]", name)))) {
+    TRUE
+  } else if (length(model) > 1) {
+    any(vapply(X = model, FUN = tar_nlmixr_multimodel_has_self_reference_single, FUN.VALUE = TRUE, name = name))
+  } else {
+    FALSE
+  }
+}
+
+tar_nlmixr_multimodel_remove_self_reference <- function(model_list, name_map) {
+  lapply(X = model_list, FUN = tar_nlmixr_multimodel_remove_self_reference_single, name_map = name_map)
+}
+
+tar_nlmixr_multimodel_remove_self_reference_single <- function(model, name_map) {
+  if (length(model) <= 1) {
+    # Do not modify it or recurse, return `model` unchanged.  Use less than or
+    # equal to in case of NULL or another zero-length object.
+  } else {
+    mask_template_match <-
+      vapply(
+        X = lapply(X = names(name_map), FUN = str2lang),
+        FUN = rxode2::.matchesLangTemplate,
+        FUN.VALUE = TRUE,
+        x = model
+      )
+    if (any(mask_template_match)) {
+      # Use the fitsimple version of the model fitting so that it is not
+      # dependent on data changes.
+      model <- str2lang(paste0(name_map[[which(mask_template_match)]], "_fitsimple"))
+    } else {
+      for (idx in seq_along(model)) {
+        model[[idx]] <-
+          tar_nlmixr_multimodel_remove_self_reference_single(
+            model = model[[idx]],
+            name_map = name_map
+          )
+      }
+    }
+  }
+  model
+}
+
+#' Generate a single nlmixr multimodel target set for one model
+#'
+#' @inheritParams tar_nlmixr_multimodel
+#' @inheritParams tar_nlmixr
+#' @keywords Internal
 tar_nlmixr_multimodel_single <- function(object, name, data, est, control, table, env) {
-  # Hash the model itself without its description.  Then, if the description
-  # changes, the model will not need to rerun.
-  hash_long <- digest::digest(eval(object, envir = env))
+  # Trade-off: Running digest() on the call (object) will rerun the model if the
+  # function name changes even if the underlying model does not change.  Running
+  # digest on the evaluated call (eval(object, envir = env)) will not rerun if
+  # the function name changes, but the cost of evaluation could be large if the
+  # user puts a lot of information into generating the object (e.g. lots of
+  # model piping).
+  #
+  # Choice: Use the computationally-cheap option here.
+  hash_long <- digest::digest(object)
   hash <- substr(hash_long, 1, 8)
   name_hash <- paste(name, hash, sep = "_")
   tar_prep <-
diff --git a/tests/testthat/test-tar_nlmixr_multimodel.R b/tests/testthat/test-tar_nlmixr_multimodel.R
index 36812d0..89d66f5 100644
--- a/tests/testthat/test-tar_nlmixr_multimodel.R
+++ b/tests/testthat/test-tar_nlmixr_multimodel.R
@@ -170,7 +170,7 @@ targets::tar_test("tar_nlmixr_multimodel works with initial condition setting `c
   expect_type(targets::tar_outdated(callr_function = NULL), "character")
 })
 
-test_that("tar_nlmixr_multimodel works for within-list model piping (#19)", {
+test_that("tar_nlmixr_multimodel works for within-list model piping (#19), direct testing", {
   pheno <- function() {
     ini({
       lcl <- log(0.008); label("Typical value of clearance")
@@ -212,7 +212,57 @@ test_that("tar_nlmixr_multimodel works for within-list model piping (#19)", {
   expect_true(grepl(x = as.character(collating_call[[3]]), pattern = "^foo_[0-9a-f]{8}$"))
   expect_equal(names(collating_call), c("", "my first model", "my second model"))
 
-  # Verify the targets created are the ones being collated
-  expect_equal(collating_call[[2]], as.name(target_list[[1]][[4]]$settings$name))
-  expect_equal(collating_call[[3]], as.name(target_list[[2]][[4]]$settings$name))
+  # Verify the dependent target is created correctly
+  expect_true(rxode2::.matchesLangTemplate(
+    x = target_list[[2]]$object_simple$command$expr[[1]],
+    template =
+      str2lang(sprintf(
+        "nlmixr_object_simplify(object = rxode2::ini(%s, lcl = log(0.01)))",
+        target_list[[1]]$fit_simple$settings$name
+      ))
+  ))
+})
+
+# targets::tar_test() runs the test code inside a temporary directory
+# to avoid accidentally writing to the user's file space.
+targets::tar_test("tar_nlmixr_multimodel works for within-list model piping (#19), testing via target creation", {
+  targets::tar_script({
+    # TODO: Remove load_all
+    devtools::load_all("c:/git/nlmixr2/nlmixr2targets/")
+    #library(nlmixr2targets)
+    pheno <- function() {
+      ini({
+        lcl <- log(0.008); label("Typical value of clearance")
+        lvc <-  log(0.6); label("Typical value of volume of distribution")
+        etalcl + etalvc ~ c(1,
+                            0.01, 1)
+        cpaddSd <- 0.1; label("residual variability")
+      })
+      model({
+        cl <- exp(lcl + etalcl)
+        vc <- exp(lvc + etalvc)
+        kel <- cl/vc
+        d/dt(central) <- -kel*central
+        cp <- central/vc
+        cp ~ add(cpaddSd)
+      })
+    }
+
+    target_list <-
+      tar_nlmixr_multimodel(
+        name = foo, data = nlmixr2data::pheno_sd, est = "saem",
+        "my first model" = pheno,
+        "my second model" = foo[["my first model"]] |> rxode2::ini(lcl = log(0.01))
+      )
+  })
+  dependencies <- targets::tar_network()$edges
+  # There is one fitsimple object (estimated model result) that generates an
+  # osimple (prepared model) object
+  expect_equal(
+    sum(
+      grepl(x = dependencies$from, pattern = "foo_.{8}_fitsimple") &
+        grepl(x = dependencies$to, pattern = "foo_.{8}_osimple")
+    ),
+    1
+  )
 })

From e9d749ac43f7cb9da3a920f539ea78fbdfd90d9c Mon Sep 17 00:00:00 2001
From: Bill Denney <wdenney@humanpredictions.com>
Date: Wed, 20 Mar 2024 10:30:03 -0400
Subject: [PATCH 3/5] Allow sequential dependencies

---
 R/tar_nlmixr_multimodel.R                     |  4 +-
 ...ar_nlmixr_multimodel_has_self_reference.Rd | 42 +++++++++++++++++
 man/tar_nlmixr_multimodel_parse.Rd            | 45 +++++++++++++++++++
 man/tar_nlmixr_multimodel_single.Rd           | 45 +++++++++++++++++++
 tests/testthat/test-tar_nlmixr_multimodel.R   | 35 +++++++++++++--
 5 files changed, 167 insertions(+), 4 deletions(-)
 create mode 100644 man/tar_nlmixr_multimodel_has_self_reference.Rd
 create mode 100644 man/tar_nlmixr_multimodel_parse.Rd
 create mode 100644 man/tar_nlmixr_multimodel_single.Rd

diff --git a/R/tar_nlmixr_multimodel.R b/R/tar_nlmixr_multimodel.R
index 84c3ccf..c770e2e 100644
--- a/R/tar_nlmixr_multimodel.R
+++ b/R/tar_nlmixr_multimodel.R
@@ -46,13 +46,15 @@ tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_l
   while (any(mask_self_referential)) {
     mask_self_referential_orig <- mask_self_referential
     model_list_self_reference <- model_list[mask_self_referential]
+    # Generate a mapping of names to their target names, only for
+    # non-self-referential models.
     name_map <-
       setNames(
         vapply(X = ret_prep, FUN = \(x) x$name, FUN.VALUE = ""),
         # rxode2::.matchesLangTemplate() treats single vs double quotes in a
         # call the same.
         sprintf("%s[['%s']]", name, names(ret_prep))
-      )
+      )[!mask_self_referential]
     model_list_fewer_self_ref <-
       tar_nlmixr_multimodel_remove_self_reference(model_list = model_list[mask_self_referential], name_map = name_map)
     # Replace self-referential models with possibly-not-self-referential models
diff --git a/man/tar_nlmixr_multimodel_has_self_reference.Rd b/man/tar_nlmixr_multimodel_has_self_reference.Rd
new file mode 100644
index 0000000..390c2da
--- /dev/null
+++ b/man/tar_nlmixr_multimodel_has_self_reference.Rd
@@ -0,0 +1,42 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/tar_nlmixr_multimodel.R
+\name{tar_nlmixr_multimodel_has_self_reference}
+\alias{tar_nlmixr_multimodel_has_self_reference}
+\alias{tar_nlmixr_multimodel_has_self_reference_single}
+\title{Does the model list refer to another model in the model list?}
+\usage{
+tar_nlmixr_multimodel_has_self_reference(model_list, name)
+
+tar_nlmixr_multimodel_has_self_reference_single(model, name)
+}
+\arguments{
+\item{name}{Symbol, name of the target. A target
+name must be a valid name for a symbol in R, and it
+must not start with a dot. Subsequent targets
+can refer to this name symbolically to induce a dependency relationship:
+e.g. \code{tar_target(downstream_target, f(upstream_target))} is a
+target named \code{downstream_target} which depends on a target
+\code{upstream_target} and a function \code{f()}. In addition, a target's
+name determines its random number generator seed. In this way,
+each target runs with a reproducible seed so someone else
+running the same pipeline should get the same results,
+and no two targets in the same pipeline share the same seed.
+(Even dynamic branches have different names and thus different seeds.)
+You can recover the seed of a completed target
+with \code{tar_meta(your_target, seed)} and run \code{\link[targets:tar_seed_set]{tar_seed_set()}}
+on the result to locally recreate the target's initial RNG state.}
+}
+\value{
+A logical vector the same length as \code{model_list} indicating if the
+model is self-referential to another model in the list
+}
+\description{
+Does the model list refer to another model in the model list?
+}
+\section{Functions}{
+\itemize{
+\item \code{tar_nlmixr_multimodel_has_self_reference_single()}: A helper function to
+look at each call for each model separately
+
+}}
+\keyword{Internal}
diff --git a/man/tar_nlmixr_multimodel_parse.Rd b/man/tar_nlmixr_multimodel_parse.Rd
new file mode 100644
index 0000000..2c8349b
--- /dev/null
+++ b/man/tar_nlmixr_multimodel_parse.Rd
@@ -0,0 +1,45 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/tar_nlmixr_multimodel.R
+\name{tar_nlmixr_multimodel_parse}
+\alias{tar_nlmixr_multimodel_parse}
+\title{Generate nlmixr multimodel target set for all models in one call to
+\code{tar_nlmixr_multimodel()}}
+\usage{
+tar_nlmixr_multimodel_parse(name, data, est, control, table, model_list, env)
+}
+\arguments{
+\item{name}{Symbol, name of the target. A target
+name must be a valid name for a symbol in R, and it
+must not start with a dot. Subsequent targets
+can refer to this name symbolically to induce a dependency relationship:
+e.g. \code{tar_target(downstream_target, f(upstream_target))} is a
+target named \code{downstream_target} which depends on a target
+\code{upstream_target} and a function \code{f()}. In addition, a target's
+name determines its random number generator seed. In this way,
+each target runs with a reproducible seed so someone else
+running the same pipeline should get the same results,
+and no two targets in the same pipeline share the same seed.
+(Even dynamic branches have different names and thus different seeds.)
+You can recover the seed of a completed target
+with \code{tar_meta(your_target, seed)} and run \code{\link[targets:tar_seed_set]{tar_seed_set()}}
+on the result to locally recreate the target's initial RNG state.}
+
+\item{data}{nlmixr data}
+
+\item{est}{estimation method (all methods are shown by
+`nlmixr2AllEst()`). Methods can be added for other tools}
+
+\item{control}{The estimation control object.  These are expected
+to be different for each type of estimation method}
+
+\item{table}{The output table control object (like
+`tableControl()`)}
+
+\item{env}{The environment where the model is setup (not needed for typical
+use)}
+}
+\description{
+Generate nlmixr multimodel target set for all models in one call to
+\code{tar_nlmixr_multimodel()}
+}
+\keyword{Internal}
diff --git a/man/tar_nlmixr_multimodel_single.Rd b/man/tar_nlmixr_multimodel_single.Rd
new file mode 100644
index 0000000..f82bef4
--- /dev/null
+++ b/man/tar_nlmixr_multimodel_single.Rd
@@ -0,0 +1,45 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/tar_nlmixr_multimodel.R
+\name{tar_nlmixr_multimodel_single}
+\alias{tar_nlmixr_multimodel_single}
+\title{Generate a single nlmixr multimodel target set for one model}
+\usage{
+tar_nlmixr_multimodel_single(object, name, data, est, control, table, env)
+}
+\arguments{
+\item{object}{Fitted object or function specifying the model.}
+
+\item{name}{Symbol, name of the target. A target
+name must be a valid name for a symbol in R, and it
+must not start with a dot. Subsequent targets
+can refer to this name symbolically to induce a dependency relationship:
+e.g. \code{tar_target(downstream_target, f(upstream_target))} is a
+target named \code{downstream_target} which depends on a target
+\code{upstream_target} and a function \code{f()}. In addition, a target's
+name determines its random number generator seed. In this way,
+each target runs with a reproducible seed so someone else
+running the same pipeline should get the same results,
+and no two targets in the same pipeline share the same seed.
+(Even dynamic branches have different names and thus different seeds.)
+You can recover the seed of a completed target
+with \code{tar_meta(your_target, seed)} and run \code{\link[targets:tar_seed_set]{tar_seed_set()}}
+on the result to locally recreate the target's initial RNG state.}
+
+\item{data}{nlmixr data}
+
+\item{est}{estimation method (all methods are shown by
+`nlmixr2AllEst()`). Methods can be added for other tools}
+
+\item{control}{The estimation control object.  These are expected
+to be different for each type of estimation method}
+
+\item{table}{The output table control object (like
+`tableControl()`)}
+
+\item{env}{The environment where the model is setup (not needed for typical
+use)}
+}
+\description{
+Generate a single nlmixr multimodel target set for one model
+}
+\keyword{Internal}
diff --git a/tests/testthat/test-tar_nlmixr_multimodel.R b/tests/testthat/test-tar_nlmixr_multimodel.R
index 89d66f5..ac6de81 100644
--- a/tests/testthat/test-tar_nlmixr_multimodel.R
+++ b/tests/testthat/test-tar_nlmixr_multimodel.R
@@ -221,15 +221,44 @@ test_that("tar_nlmixr_multimodel works for within-list model piping (#19), direc
         target_list[[1]]$fit_simple$settings$name
       ))
   ))
+
+  # Verify that circular references are caught
+  expect_error(
+    tar_nlmixr_multimodel(
+      name = foo, data = nlmixr2data::pheno_sd, est = "saem",
+      "my first model" = pheno,
+      "my second model" = foo[["my third model"]] |> rxode2::ini(lcl = log(0.01)),
+      "my third model" = foo[["my second model"]] |> rxode2::ini(lcl = log(0.1))
+    ),
+    regexp = 'The following model\\(s\\) appear to have circular references to each other: "my second model", "my third model"'
+  )
+
+  # Verify that sequential references work
+  target_list <-
+    tar_nlmixr_multimodel(
+      name = foo, data = nlmixr2data::pheno_sd, est = "saem",
+      "my first model" = pheno,
+      "my second model" = foo[["my first model"]] |> rxode2::ini(lcl = log(0.01)),
+      "my third model" = foo[["my second model"]] |> rxode2::ini(lcl = log(0.1))
+    )
+  expect_equal(length(target_list), 4)
+  # The second model depends on the first
+  expect_true(
+    target_list[[1]]$fit_simple$settings$name %in%
+      targets::tar_deps_raw(target_list[[2]]$object_simple$command$expr)
+  )
+  # The third model depends on the second
+  expect_true(
+    target_list[[2]]$fit_simple$settings$name %in%
+      targets::tar_deps_raw(target_list[[3]]$object_simple$command$expr)
+  )
 })
 
 # targets::tar_test() runs the test code inside a temporary directory
 # to avoid accidentally writing to the user's file space.
 targets::tar_test("tar_nlmixr_multimodel works for within-list model piping (#19), testing via target creation", {
   targets::tar_script({
-    # TODO: Remove load_all
-    devtools::load_all("c:/git/nlmixr2/nlmixr2targets/")
-    #library(nlmixr2targets)
+    library(nlmixr2targets)
     pheno <- function() {
       ini({
         lcl <- log(0.008); label("Typical value of clearance")

From 1b84a0d0cf3a4ebd5199833494697fe03cd16b16 Mon Sep 17 00:00:00 2001
From: Bill Denney <wdenney@humanpredictions.com>
Date: Wed, 20 Mar 2024 10:41:11 -0400
Subject: [PATCH 4/5] Fix check issues

---
 R/tar_nlmixr_multimodel.R                       | 4 +++-
 man/tar_nlmixr_multimodel_has_self_reference.Rd | 4 ++++
 man/tar_nlmixr_multimodel_parse.Rd              | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/R/tar_nlmixr_multimodel.R b/R/tar_nlmixr_multimodel.R
index c770e2e..f47d21d 100644
--- a/R/tar_nlmixr_multimodel.R
+++ b/R/tar_nlmixr_multimodel.R
@@ -27,6 +27,7 @@ tar_nlmixr_multimodel <- function(name, ..., data, est, control = list(), table
 #'
 #' @inheritParams tar_nlmixr_multimodel
 #' @inheritParams tar_nlmixr
+#' @param model_list A named list of calls for model targets to be created
 #' @keywords Internal
 tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_list, env) {
   checkmate::assert_named(model_list, type = "unique")
@@ -49,7 +50,7 @@ tar_nlmixr_multimodel_parse <- function(name, data, est, control, table, model_l
     # Generate a mapping of names to their target names, only for
     # non-self-referential models.
     name_map <-
-      setNames(
+      stats::setNames(
         vapply(X = ret_prep, FUN = \(x) x$name, FUN.VALUE = ""),
         # rxode2::.matchesLangTemplate() treats single vs double quotes in a
         # call the same.
@@ -113,6 +114,7 @@ tar_nlmixr_multimodel_has_self_reference <- function(model_list, name) {
 }
 #' @describeIn tar_nlmixr_multimodel_has_self_reference A helper function to
 #'   look at each call for each model separately
+#' @param model A single model call for the model target to be created
 tar_nlmixr_multimodel_has_self_reference_single <- function(model, name) {
   if (rxode2::.matchesLangTemplate(model, str2lang(sprintf("%s[[.]]", name)))) {
     TRUE
diff --git a/man/tar_nlmixr_multimodel_has_self_reference.Rd b/man/tar_nlmixr_multimodel_has_self_reference.Rd
index 390c2da..5ae6fbd 100644
--- a/man/tar_nlmixr_multimodel_has_self_reference.Rd
+++ b/man/tar_nlmixr_multimodel_has_self_reference.Rd
@@ -10,6 +10,8 @@ tar_nlmixr_multimodel_has_self_reference(model_list, name)
 tar_nlmixr_multimodel_has_self_reference_single(model, name)
 }
 \arguments{
+\item{model_list}{A named list of calls for model targets to be created}
+
 \item{name}{Symbol, name of the target. A target
 name must be a valid name for a symbol in R, and it
 must not start with a dot. Subsequent targets
@@ -25,6 +27,8 @@ and no two targets in the same pipeline share the same seed.
 You can recover the seed of a completed target
 with \code{tar_meta(your_target, seed)} and run \code{\link[targets:tar_seed_set]{tar_seed_set()}}
 on the result to locally recreate the target's initial RNG state.}
+
+\item{model}{A single model call for the model target to be created}
 }
 \value{
 A logical vector the same length as \code{model_list} indicating if the
diff --git a/man/tar_nlmixr_multimodel_parse.Rd b/man/tar_nlmixr_multimodel_parse.Rd
index 2c8349b..9d3bb7d 100644
--- a/man/tar_nlmixr_multimodel_parse.Rd
+++ b/man/tar_nlmixr_multimodel_parse.Rd
@@ -35,6 +35,8 @@ to be different for each type of estimation method}
 \item{table}{The output table control object (like
 `tableControl()`)}
 
+\item{model_list}{A named list of calls for model targets to be created}
+
 \item{env}{The environment where the model is setup (not needed for typical
 use)}
 }

From fef8a9434926e9b3e0205cb5e01661090033bc37 Mon Sep 17 00:00:00 2001
From: Bill Denney <wdenney@humanpredictions.com>
Date: Wed, 20 Mar 2024 10:53:54 -0400
Subject: [PATCH 5/5] Update vignette for piping

---
 vignettes/estimating.Rmd | 52 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/vignettes/estimating.Rmd b/vignettes/estimating.Rmd
index 5bec88c..fe17d14 100644
--- a/vignettes/estimating.Rmd
+++ b/vignettes/estimating.Rmd
@@ -155,3 +155,55 @@ list(
   plan_report
 )
 ```
+
+## Model piping for multiple models estimated with one dataset
+
+Model piping for `nlmixr2` models (see
+`vignette("modelPiping", package = "nlmixr2")`) is possible within the multiple
+models being estimated with `tar_nlmixr_multimodel()`.  It simplifies examples
+like the one above so that you can focus on the model content and avoid
+rewriting models, as with all `nlmixr2` model piping.
+
+To use model piping, simply refer to the model by its name like a named list.
+Behind the scenes, `nlmixr2targets` will work out the dependencies between the
+models and only rerun the dependent model if it or the dependent model changes.
+
+```{r piping-tar_nlmixr_multimodel, eval = FALSE}
+library(targets)
+library(tarchetypes)
+library(nlmixr2targets)
+library(nlmixr2)
+
+pheno <- function() {
+  ini({
+    lcl <- log(0.008); label("Typical value of clearance")
+    lvc <-  log(0.6); label("Typical value of volume of distribution")
+    etalcl + etalvc ~ c(1,
+                        0.01, 1)
+    cpaddSd <- 0.1; label("residual variability")
+  })
+  model({
+    cl <- exp(lcl + etalcl)
+    vc <- exp(lvc + etalvc)
+    kel <- cl/vc
+    d/dt(central) <- -kel*central
+    cp <- central/vc
+    cp ~ add(cpaddSd)
+  })
+}
+
+plan_model <-
+  tar_nlmixr_multimodel(
+    all_models,
+    data = nlmixr2data::pheno_sd,
+    est = "saem",
+    "Base model; additive residual error = 1" = pheno,
+    "Base model; additive residual error = 3" =
+      all_models[["Base model; additive residual error = 1"]] |>
+      ini(cpaddSd = 3)
+  )
+
+list(
+  plan_model
+)
+```