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

Add reset functions to evtools #1222

Merged
merged 6 commits into from
Aug 3, 2024
Merged

Add reset functions to evtools #1222

merged 6 commits into from
Aug 3, 2024

Conversation

kylebaron
Copy link
Collaborator

@kylebaron kylebaron commented Aug 1, 2024

Summary

Trying to follow a similar pattern to what we do for replace, bolus and friends; this should (hopefully) all be natural / consistent riffing on the evtools machinery that is already in place.

  • Reset sends EVID 3; it resets the entire PK system (all compartments)
  • We have to use some value for compartment; keeping that at the default value of 1
  • Providing reset() functions with self as the first argument (the event is sent off right away, no return) and without self as first argument (you get the object back, with opportunity to interact with it)
  • Also providing reset() with option to pass along dosing information; this is EVID 4 in NONMEM and mrgsolve; it resets the system and then doses
  • For reset() with dose, I'm tacking on rate as an optional argument, defaulting to 0 (or bolus); this is a slight difference from what we've done before in that we don't provide separate functions to reset and bolus or reset and infuse
  • You can always ask for an infusion dose, update the evid to 4 and get the same result

I also went back into some of the previously-developed functions and used explicit evt:: references to functions inside the evtools namespace. I got a little spooked adding a function with a vanilla name like reset() and wanted to get explicit about all of this in the code.

Tests

You can see from Files Changed that tests are in inst/maintenance/unit-cpp/test-evtools.R. This can be run with make test-cpp from the command line.

The tests are a little tedious, trying to figure out when the system was reset and when there was a dose. I thought to run the equivalent simulation with no evtools involvement and verify results later on in the process; I think this is pretty compelling, but kept the more brute force tests anyway; probably still useful for catching something that might go wrong some day.

@kylebaron kylebaron requested a review from kyleam August 2, 2024 12:44
Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a little spooked adding a function with a vanilla name like reset() and wanted to get explicit about all of this in the code.

Makes sense.


We have to use some value for compartment; setting that to 1

Checking my understanding. The compartment is not explicitly set to 1 in reset, correct? It's just that it's left at the default value.

out2 <- mrgsim(mod, param = list(mode = 13))
expect_identical(as.data.frame(out1), as.data.frame(out2))

# Do an equivalent simulation with no evtools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@kylebaron
Copy link
Collaborator Author

The compartment is not explicitly set to 1 in reset, correct? It's just that it's left at the default value.

Yeah, that's right. The description I left was misleading on intent - I didn't have it clear in my own head at first. But now I think that's the only reasonable thing to do; and it's consistent with how ev() objects operate on the R side.

Thanks for reviewing this.

@kylebaron kylebaron merged commit cdea044 into main Aug 3, 2024
7 checks passed
@kylebaron kylebaron deleted the evtools-reset branch August 3, 2024 19:28
@kylebaron kylebaron linked an issue Sep 1, 2024 that may be closed by this pull request
@kylebaron kylebaron mentioned this pull request Oct 17, 2024
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.

Add reset functionality to evtools, with optional doses
2 participants