nlmixrdevelopment / RxODE

RxODE is an R package that facilitates easy simulations in R
https://nlmixrdevelopment.github.io/RxODE/
GNU General Public License v3.0
55 stars 14 forks source link

mrgsolve differences #336

Closed mattfidler closed 3 years ago

mattfidler commented 3 years ago

In the upcoming release I have the following paragraph.

mrgsolve, which uses a thread-safe C++ lsoda solver to solve ODE systems. The user is required to write hybrid R/C++ code to create a mrgsolve model.

In contrast, RxODE has a R-like mini-language that is parsed into C code that solves the ODE system.

Both mrgsolve and RxODE implement thread-safe ODE solving. In RxODE threading for each individual ODE solved is turned on by default and the supported RxODE functions are all thread-safe. Not all functions in mrgsolve are thread safe currently, so caution should be used when using some functions (like random number generation) or using custom functions.

Unlike RxODE, mrgsolve does not currently support symbolic manipulation of ODE systems, like automatic Jacobian calculation or forward sensitivity calculation (RxODE currently supports this and this is the basis of nlmixr’s FOCEi algorithm)

@billdenney or @kylebaron do you think this is accurate? Particularly, I don't know if you want to mention thread safe since I'm unsure how well tested this is.

kylebaron commented 3 years ago

Hi @mattfidler -

I'd say that in mrgsolve, the whole mode is written in C++; it doesn't call back to R for anything, but rather we translate the model into C++ code to solve the ODE system.

mrgsolve never attempts to multithread anything. What were you thinking about for dangerous code that warrants caution there?

Kyle

billdenney commented 3 years ago

@mattfidler, these details are outside of my expertise.

mattfidler commented 3 years ago

Thanks @kylebaron

I'm not sure what you mean by:

What were you thinking about for dangerous code that warrants caution there?

If you mean thread-safe, you have to look at the R source and the Rcpp source to see if the containers/functions are thread safe. Rcpp sometimes calls R, even though you may not think it.

mattfidler commented 3 years ago

You mean this piece:

Not all functions in mrgsolve are thread safe currently, so caution should be used when using some functions (like random number generation) or using custom functions.

Since you are not using threading, this is unimportant and will be removed.

One last point @kylebaron, you agree with this statement:

mrgsolve currently has 1-2 compartment (super-positioning) models built-in. The solved systems and ODEs cannot currently be mixed. Since this uses super-positioning, time-varying covariates are not supported/solved correctly.

mattfidler commented 3 years ago

If you have any comments let me know. For now I'm keeping the statement. It is supported here https://pubmed.ncbi.nlm.nih.gov/25841670/

kylebaron commented 3 years ago

Since this uses super-positioning, time-varying covariates are not supported/solved correctly.

Does the paper say that mrgsolve uses super position?

mattfidler commented 3 years ago

No; When I looked at your code you had the exponential super positioning for linear compartmental models, is that not correct?

I couldn't tell if you could mix ODEs and linear compartment solutions, but I saw no examples, so I assumed it wasn't supported.

kylebaron commented 3 years ago

Right; we can't mix ode and closed-form solutions. But I believe we can simulate time-varying data. Can you share an example where you think the simulation is wrong?

mattfidler commented 3 years ago

To be clear, this is on principle, I haven't taken the time to work out an example; However, if you use super-positioning it will not match NONMEM (this is according to the paper cited above).

The examples are in the paper cited above. They found that super-positioning did not match NONMEM's output; Hence they deemed their solutions the "ADVAN" solution since they match NONMEM's output while super-positioning did not.

mattfidler commented 3 years ago

Note that the ADVAN solution matched the ODE solution, so it is deemed to be correct.

kylebaron commented 3 years ago

Does this count? https://github.com/mrgsolve/nmtests/blob/master/nmtest_time_varying.md#concentration-versus-time

kylebaron commented 3 years ago

Is that what you're talking about for time-varying covariates ?

mattfidler commented 3 years ago

I think they used more extreme examples.

kylebaron commented 3 years ago

Ok; what example would you find convincing? Point me to the model.

mattfidler commented 3 years ago

@abuhelwa Can you provide the examples you used in your paper?

mattfidler commented 3 years ago

To be clear @kylebaron, I believe that mrgsolve is a validated and excellent tool; At the same time, I believe that super-positioning may not always be appropriate.

kylebaron commented 3 years ago

This should mimic at least the magnitude of their example: https://github.com/mrgsolve/nmtests/blob/master/nmtest_time_varying-2.md#concentration-versus-time

mattfidler commented 3 years ago

True.

kylebaron commented 3 years ago

@mattfidler The code that mrgsolve uses was adopted from code in Torsten and that code came from BUGS model library that BIll Gillespie worked on. It uses some poly exponential calculations. I don't believe it depends on super-positioning. If @abuhelwa's paper is the benchmark then I think we can say it can handle time-varying covariates and give an accurate answer. But - as always - I'm happy to know about any situation where you can show that the answer isn't right using master branch code (we did get a report of some gremlins with certain input / models and we were grateful for the report and promptly addressed the issue). Thanks.

mattfidler commented 3 years ago

Ok thanks. I'll update it.

On Wed, Nov 25, 2020, 7:52 AM Kyle Baron notifications@github.com wrote:

@mattfidler https://github.com/mattfidler The code that mrgsolve uses was adopted from code in Torsten and that code came from BUGS model library that BIll Gillespie worked on. It uses some poly exponential calculations. I don't believe it depends on super-positioning. If @abuhelwa https://github.com/abuhelwa's paper is the benchmark then I think we can say it can handle time-varying covariates and give an accurate answer. But - as always - I'm happy to know about any situation where you can show that the answer isn't right using master branch code (we did get a report of some gremlins with certain input / models and we were grateful for the report and promptly addressed the issue). Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nlmixrdevelopment/RxODE/issues/336#issuecomment-733718833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5VWTVBTGQ64NDUR2NWTLSRUDYRANCNFSM4T54PXIA .