nlmixr2 / nlmixr2lib

https://nlmixr2.github.io/nlmixr2lib/
5 stars 3 forks source link

Added model attributes #73

Closed john-harrold closed 11 hours ago

john-harrold commented 1 week ago

The documentation was updated to include

The model database table (modeldb) was updated to include:

I updated the tests to correct some hard references to list positions that were shifted when I updated the model R scripts in inst.

billdenney commented 1 week ago

Thanks for creating this. A few thoughts:

  1. I see "linCmt" as a special case of algebraic solutions. It is an important special case for nlmixr2. That said, would it make sense to call it "algebraic" so that a generic Emax model could also be true? (I'm open to either position here, it just struck me as specific.)
  2. I think that you can autogenerate the depends column based on the covariate list from the model. Is it doing something else? If not, I would suggest replacing it with the covariate list.
john-harrold commented 1 week ago

My main motivation here is to be able to include the model library in ruminate to do rule-based dosing. To do that I need a couple things. I cannot stop and start models that have solved components, only with ODEs. So I added that column to allow me to filter the modeldb based on that. The way I'm picking it up is using the params field of the model object. It's not a user specified model attribute. See the example in this comment:

https://github.com/nlmixr2/nlmixr2lib/issues/69#issuecomment-2168232459

I don't know of a way to detect a purely algebraic model. If you do, I'm happy to do that because it would make it more generalizable.

I need the pieces feeding in from other models separately from the covariates. I want to allow users to select models as base models to build on them. So I need to know if the model has something other than covariates to exclude it from being a base model. And then I want to take the elements of the current model and see if they satisfy the depends for others to allow them to be stacked/piped together.

I'm handling covariates separately. Those can be constructed by the user. This shows how it's done programmatically but there is also an interface as well:

https://ruminate.ubiquity.tools/articles/clinical_trial_simulation.html#covariates

mattfidler commented 5 days ago

I don't know of a way to detect a purely algebraic model. If you do, I'm happy to do that because it would make it more generalizable.

Yes there is:

library(rxode2)
#> Warning: package 'rxode2' was built under R version 4.3.3
#> rxode2 2.1.3 using 4 threads (see ?getRxThreads)
#>   no cache: create with `rxCreateCache()`
mod <- function() {
  ini({
    e0 <- 0.1
    em <- 10
    e50 <- 20
    err.sd <- 0.1
  })
  model({
    ef <- e0 + (em - e0) * time / (time + e50)
    ef ~ add(err.sd)
  })
}

mod <- mod()

!mod$params$linCmt && (length(mod$params$cmt)  == 0)
#> [1] TRUE

mod <- function() {
  ini({
    ka <- 1
    cl <- 0.1
    vc <- 4
    err.sd <- 0.1
  })
  model({
    Cc <- linCmt(ka, cl, vc)
    Cc ~ add(err.sd)
  })
}

mod <- mod()

!mod$params$linCmt && (length(mod$params$cmt)  == 0)
#> [1] FALSE

mod <- function() {
  ini({
    cl <- 0.1
    vc <- 4
    err.sd <- 0.1
  })
  model({
    d/dt(central) <- -central * cl / vc
    Cc <- central / vc
    Cc ~ add(err.sd)
  })
}

mod <- mod()

!mod$params$linCmt && (length(mod$params$cmt)  == 0)
#> [1] FALSE

Created on 2024-06-23 with reprex v2.1.0

Also found and fixed a bug with this regexp, could be triggered by your project

https://github.com/nlmixr2/rxode2/pull/697

john-harrold commented 5 days ago

@mattfidler is it possible to have linCmt() used with ODES?

mattfidler commented 5 days ago

Yes. It is possible though not well tested

On Sun, Jun 23, 2024, 4:20 PM John Harrold @.***> wrote:

Thankyou Matt. Bill let me know if there is anything else you'd like for me to change? I can make changes that take what Matt described above into account, and I can add a column called algebraic.

@mattfidler https://github.com/mattfidler is it possible to have linCmt() used with ODES?

— Reply to this email directly, view it on GitHub https://github.com/nlmixr2/nlmixr2lib/pull/73#issuecomment-2185009098, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5VWWRDFXTVKMJ4CNVN3LZI3KR3AVCNFSM6AAAAABJW5NCKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGAYDSMBZHA . You are receiving this because you were mentioned.Message ID: @.***>

john-harrold commented 5 days ago

But the mod$params$linCmt field will be TRUE if that is the case?

mattfidler commented 5 days ago

Yes.

billdenney commented 5 days ago

Given these suggestions, maybe there should be two columns. One could be "ode" and one "linCmt". Then, all the information is there to use.

The only other change would be to fix the CodeFactor issues, please.

Other than that, I think we're good.

john-harrold commented 5 days ago

Yeah I was going to try and address the code factor issues. Is it because I'm using c() with a single element?

billdenney commented 5 days ago

Yeah, that's it.

john-harrold commented 4 days ago

One other question. Should a model be flagged as algebraic if it has no ODES or if it has no ODES and no linCmt()?

billdenney commented 4 days ago

To me, if it has no ODEs, it's algebraic because linCmt is also an algebraic solution.

mattfidler commented 4 days ago

To me, if it has no ODEs, it's algebraic because linCmt is also an algebraic solution.

Well to me the difference between linCmt and algebraic is the ability to dose the central and depot compartments, so I think they are a bit different.

john-harrold commented 3 days ago

I think the two of you can see why I'm going back and forth on this :). I think it makes sense to use Bills method. Then if you just want purely algebraic you can filter for where the algebraic column is TRUE and the linCmt is FALSE.

john-harrold commented 11 hours ago

Ok I had to make a command decision so I can move on in life :). I went with Matts suggestion. So linCmt() is true if there are any linCmt() commands. And the algebraic column is only true for purely algebraic (no linCmt() and no ODEs) models.

billdenney commented 11 hours ago

That works. :)

To me, there are two states to define which may overlap:

As long as these are possible to detect, then I'm good with whatever solution is proposed.

I'll review the code now.

billdenney commented 11 hours ago

Looks good to me!