nlmixrdevelopment / nlmixr

nlmixr: an R package for population PKPD modeling
https://nlmixrdevelopment.github.io/nlmixr/
GNU General Public License v2.0
116 stars 45 forks source link

find.theta not checking for transformed thetas #107

Closed jprybylski closed 6 years ago

jprybylski commented 6 years ago

I recently built a large PBPK model for nlmixr, so needless to say I expected issues. I got the dreaded "Error parsing model" and after a day of trace with editing in nlmixrUIModel, I found the issue was in the call to find.theta at https://github.com/nlmixrdevelopment/nlmixr/blob/3c71a63c2103a65bf35389225dae95893b7799cb/R/ui.R#L1031 My x[[wm0]] is indeed a theta, but it is the transformation CL.Li.Fe.Gd <- exp(lCL.Li.Fe.Gd+eta.CL.Li.Fe.Gd) Because it is not the exact name "lCL.Li.Fe.Gd", find.theta takes it to the next condition, which tries to call it as "x[[1]]," leading to the error that is causing my "Error parsing model" result: Error in x[[1]] : object of type 'symbol' is not subsettable

Is this code running as intended, or should the UI be modified to recognize modified thetas?

jprybylski commented 6 years ago

Actually, I noticed this could be resolved by changing https://github.com/nlmixrdevelopment/nlmixr/blob/3c71a63c2103a65bf35389225dae95893b7799cb/R/ui.R#L641 to } else if (length(x) > 1 && identical(x[[1]], quote('+'))){

Although I still wonder if find.theta should recognize transformed thetas

mattfidler commented 6 years ago

Hi @jprybylski

I'm impressed by your R debugging skills! I pushed the fix earlier, but I am also concerned if it is recognizing the transformed thetas. I cannot personally debug the model without the model. If you want to look at the model off the forum, you could send it to me in my personal email.

Another option is to look yourself. Say you named your pbpk model, you can do the following:

tmp <- nlmixr(pbpk)

Then (using the theo example)

> tmp
▂▂ 1-compartment model with first-order absorption in terms of Cl ▂▂▂▂▂▂▂▂▂▂▂▂▂▂
── Initialization: ─────────────────────────────────────────────────────────────
Fixed Effects ($theta):
 tka  tcl   tv 
 0.5 -3.2 -1.0 

Omega ($omega):
       eta.ka eta.cl eta.v
eta.ka      1      0     0
eta.cl      0      2     0
eta.v       0      0     1
── Model: ──────────────────────────────────────────────────────────────────────
        ka <- exp(tka + eta.ka)
        cl <- exp(tcl + eta.cl)
        v <- exp(tv + eta.v)
▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂

You can then see what thetas are parsed and what omegas are parsed.

rexjohnrx commented 6 years ago

Wow, thanks! It's not recognizing transformed thetas, which I thought was the problem, leading to a "fix" forcing it to recognize the transformed thetas. Anyway, the fix you pushed was actually the needed change, so feel free to close this, too. (I mistakenly used an older account when opening this issue)

mattfidler commented 6 years ago

Ah. Ok.