Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added model attributes #73

Merged
merged 7 commits into from
Jun 29, 2024
Merged

Added model attributes #73

merged 7 commits into from
Jun 29, 2024

Conversation

john-harrold
Copy link
Contributor

The documentation was updated to include

  • Dosing compartments
  • Units (time and dosing)
  • A depends field for models that are intended to be built upon other models

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

  • A column indicating if a solved model has been used (linCmt)
  • Dosing states (similar to how parameters are currently used)
  • Depends (similar to how parameters are currently being used)

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

Added dependencies to oncology models
Added units to relevant models
Fixing tests that were broken by adding metadata fields.
@john-harrold john-harrold mentioned this pull request Jun 22, 2024
4 tasks
@billdenney
Copy link
Contributor

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
Copy link
Contributor Author

john-harrold commented Jun 22, 2024

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:

#69 (comment)

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
Copy link
Member

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

nlmixr2/rxode2#697

@john-harrold
Copy link
Contributor Author

john-harrold commented Jun 23, 2024

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

@mattfidler
Copy link
Member

mattfidler commented Jun 23, 2024 via email

@john-harrold
Copy link
Contributor Author

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

@mattfidler
Copy link
Member

Yes.

@billdenney
Copy link
Contributor

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
Copy link
Contributor Author

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

@billdenney
Copy link
Contributor

Yeah, that's it.

@john-harrold
Copy link
Contributor Author

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
Copy link
Contributor

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

@mattfidler
Copy link
Member

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
Copy link
Contributor Author

john-harrold commented Jun 25, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

That works. :)

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

  • linCmt or not
  • ODE or algebraic

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

I'll review the code now.

@billdenney
Copy link
Contributor

Looks good to me!

@billdenney billdenney merged commit 76640a5 into nlmixr2:main Jun 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants