metrumresearchgroup / nmrec

R package to read, parse, and modify NONMEM control records
https://metrumresearchgroup.github.io/nmrec/
Other
1 stars 0 forks source link

Ability to access the size/indices of individual records (for matrix-type records) #32

Closed barrettk closed 9 months ago

barrettk commented 9 months ago

Using this method, we can access the size of each record:

ctl <- nmrec::read_ctl(get_model_path(.mod))
pinfo <- nmrec:::create_param_index(ctl, "omega")
> pinfo$details
[[1]]
[[1]]$type
[1] "diagonal"

[[1]]$subtype
[1] "implicit"

[[1]]$size
[1] 2

[[1]]$popts
...truncated...

[[2]]
[[2]]$type
[1] "block"

[[2]]$subtype
[1] "plain"

[[2]]$size
[1] 3

[[2]]$popts
...truncated...

In helping to address https://github.com/metrumresearchgroup/bbr/issues/640, we need a method for separating the full omega matrix into individual matrices (see this comment). The full procedure for addressing the issue is outlined in this comment. As you can see in the setup section however, I hard coded the individual matrices; shown again below:

omega_full <- get_initial_est(.mod)$omegas
omega_1 <- omega_full[1:2, 1:2]
omega_2 <- omega_full[3:5, 3:5]

There are a number of ways I could get those dimensions.

In terms of how and what information we would return from the nmrec side, I would 100% leave that up to you, based on what you think nmrec should be responsible for. In the end, I just need a method for separating the matrices using exported nmrec functions, and the minimum information I need to do that, is how many rows/columns each record spans, along with the order they show up in in the control stream file (i.e. to determine the indices the record spans within the full compiled matrix).

barrettk commented 9 months ago

As an aside: In the end we need to be able to put the matrices back together. I had messed around with this pattern:

# Get full matrix
omega_full <- bbr::get_initial_est(.mod)$omegas

# Separate the full matrix. Hard code example:
omega_1 <- omega_full[1:2, 1:2]
omega_2 <- omega_full[3:5, 3:5]

# Checks that the individual matrices are positive-definite, and corrects them if need be
omega_1 <- bbr::check_and_modify_pd(omega_1)
omega_2 <- bbr::check_and_modify_pd(omega_2)

# combine matrices back into full matrix
omega_full_pd <- as.matrix(Matrix::bdiag(list(omega_1, omega_2)))

However, I really like the handling of nmrec:::vector_to_matrix_ltri. I know it doesnt make sense to export that function, so just wondering if you have any suggestions here, or think something on the nmrec side (exported) might make sense for doing this (assuming not).

kyleam commented 9 months ago

@barrettk Thanks for posting this issue.

The simplest method I can think of from an nmrec perspective, would be to have a helper to pull out the size attribute, and then I can determine the indices on the bbr front [...]

I agree that that's the simplest given the current implementation. In hopes that this doesn't block you, I've pushed a scratch implementation to scratch/extra-param-sizes.

I'm undecided on where we should end up eventually, but it probably makes sense to wait until we have a better idea of how metrumresearchgroup/bbr#640 settles.

barrettk commented 9 months ago

Awesome thanks! Will give it a shot and let you know how it goes

barrettk commented 9 months ago

Hey @kyleam While the original intention for this feature was for matrix handling (see linked issue above), I also made use of it for tabulating the record number in the new initial_estimates() function.

The matrix handling, along with tweaking initial omegas and sigmas altogether, will be handled in a separate PR.

However, after discussing with @seth127, we agreed it would be nice to include this metadata (record_number column) in this PR (Note that it wont be that exact PR, as Im going to start with a clean branch). This new column is potentially helpful for distinguishing non-informative priors, which is in line with some of the other adjustments we've made.