RXR-2394: support estimation of lagtime#51
Merged
roninsightrx merged 6 commits intomasterfrom Jul 13, 2025
Merged
Conversation
roninsightrx
commented
Jul 8, 2025
| t_init = 0, | ||
| calc_ofv, | ||
| regimen, | ||
| lagtime = c(0), |
Contributor
Author
There was a problem hiding this comment.
This is the default for PKPDsim. It needs to be non-NULL. If we pass a NULL value it will also be converted to c(0) in PKPDsim.
| @@ -0,0 +1,10 @@ | |||
| mod_1cmt_oral_lagtime <- PKPDsim::new_ode_model( | |||
Contributor
Author
There was a problem hiding this comment.
moved this out of the single test, so it can be re-used.
| ## check that fit works for vector of lagtimes | ||
| lagtimes <- c(0.25, 0.5, 0.75, 1.0, 2.0) | ||
| est <- c() | ||
| for(i in seq(lagtimes)) { |
Contributor
Author
There was a problem hiding this comment.
loop over a range of lagtime estimates (true value is 0.83). Should give estimate for all values. This in contrast to the default optimization method (BGFS) which will error for values lower than ~0.85.
jasmineirx
approved these changes
Jul 8, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, it was not possible to estimate lagtime as a parameter, since it was incorporated in the static event tables that were input to PKPDsim. We have now pulled this out of the event table and it is now handled dynamically within PKPDsim's core function. In principle, it requires just a few minor updates to PKPDmap to support estimation of the parameter: just passing the lagtime vector as argument.
However, it does introduce a second problem we need to solve, in that lagtimes are step-functions and hence introduce inconcistencies that make estimation of the parameter difficult / impossible with gradient-based optimization methods like BFGS. In practice, if the initial estimate of lagtime is lower than the data suggests, it will never be able to recover because the initial gradients are zero for TLAG. The issue can however be solved fairly easily by switching to a non-gradient-based optimization method like Nelder-Mead. I've chosen not to make this a forced or automatic switch, but to leave it to the user. I have left some info in documentation and code comments.
Related PR in PKPDsim
CI WILL ONLY PASS AFTER THE PKPDsim PR HAS BEEN MERGED