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 replace() function to evtools #1203

Merged
merged 4 commits into from
Jul 8, 2024
Merged

Add replace() function to evtools #1203

merged 4 commits into from
Jul 8, 2024

Conversation

kylebaron
Copy link
Collaborator

@kylebaron kylebaron commented Jun 22, 2024

Summary

To date, the evtools plugin has been able to quickly create and give doses given as bolus() or infuse().

This PR implements an equivalent function to replace() the amount in a specific compartment with a new amount. This is accomplished via evid 8 (already-existing functionality).

The basic setup is similar to bolus() and infuse(): we provide two overloaded replace() functions, one that gets the self object first (this will create and "send" the dose in one function call) and another replace() function that will create the object and return it to the user for further config.

@@ -49,7 +49,7 @@ struct resim {
};

struct evdata {
evdata(double a_, int b_) : time(a_), evid(b_) {
evdata(double a_, int b_) : time(a_), evid(b_) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a lint.

@@ -34,6 +34,21 @@ void infuse(databox& self, const double amt, const int cmt, const double rate) {
return;
}

mrgsolve::evdata replace(const double amt, const int cmt) {
mrgsolve::evdata ev(0, 8);
Copy link
Collaborator Author

@kylebaron kylebaron Jun 22, 2024

Choose a reason for hiding this comment

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

The signature for the constructor is time and evid. We use 8 here because that signals to zero out the compartment and add the new amount. The time value (0) is arbitrary b/c we'll set the object to get implemented now later on.

@kylebaron kylebaron changed the title Add replace function to evtools Add replace() function to evtools Jun 22, 2024
@kylebaron kylebaron requested a review from kyleam July 2, 2024 16:31
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.

Nice

after <- filter(a, time >= 5)
expect_true(all(after$C==50))

#' When the replacement is timed into the future, we see the
Copy link
Contributor

Choose a reason for hiding this comment

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

extreme nit-pick: I think it'd be better to use plain comments rather than roxygen-style ones. (I've been allergic to unnecessary roxygen-style comments ever since I spent time debugging this issue.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, @kyleam; I reverted the comments and will try to stick to plain R comments going forward (I think I do most of the time, but I'm sure there's tons of #' comments in the code too.)

@kylebaron kylebaron added this to the 1.4.3 milestone Jul 3, 2024
@kylebaron kylebaron merged commit 44019da into main Jul 8, 2024
7 checks passed
@kylebaron kylebaron deleted the evtools-replace branch July 8, 2024 12:49
@kylebaron kylebaron mentioned this pull request Jul 25, 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.

None yet

2 participants