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

Fix n and length of rate vector in regimens joined by t_dose_update #86

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

karawoo
Copy link
Contributor

@karawoo karawoo commented Jan 6, 2024

When joining regimens using t_dose_update, only the rate values from the first regimen were included, and the value in n was incorrect. This fixes both.

Copy link
Contributor

@jasmineirx jasmineirx left a comment

Choose a reason for hiding this comment

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

should we also update the $rate and $n fields for all the other joins in this function?

@karawoo
Copy link
Contributor Author

karawoo commented Jan 8, 2024

I thought this was already working for the other branches of the logic because the following works:

library("PKPDsim")
reg1 <- new_regimen(
  amt = 1,
  times = c(0, 24),
  type = "oral"
)
reg2 <- new_regimen(
  amt = 2,
  time = c(0, 24, 48),
  type = "oral"
)
reg_b <- join_regimen(reg1, reg2, dose_update = 2)
str(reg_b)
#> List of 8
#>  $ interval       : num 24
#>  $ n              : int 4
#>  $ type           : chr [1:4] "oral" "oral" "oral" "oral"
#>  $ t_inf          : num [1:4] 0 0 0 0
#>  $ dose_times     : num [1:4] 0 24 48 72
#>  $ dose_amts      : num [1:4] 1 2 2 2
#>  $ rate           : num [1:4] 0 0 0 0
#>  $ first_dose_time: POSIXct[1:1], format: "2024-01-08 18:05:03"
#>  - attr(*, "class")= chr [1:2] "regimen" "list"

Created on 2024-01-08 with reprex v2.0.2

But actually, it only works by filling in the default rate of 0 because these are oral regimens. However when I tried to update the logic, I realized that new_regimen() actually does not return rate at all for infusion regimens:

library("PKPDsim")
str(new_regimen(amt = 1, times = 0, type = "infusion", rate = 0.5))
#> List of 7
#>  $ interval       : num 24
#>  $ n              : int 1
#>  $ type           : chr "infusion"
#>  $ t_inf          : num 2
#>  $ dose_times     : num 0
#>  $ dose_amts      : num 1
#>  $ first_dose_time: POSIXct[1:1], format: "2024-01-08 18:04:32"
#>  - attr(*, "class")= chr [1:2] "regimen" "list"

Created on 2024-01-08 with reprex v2.0.2

So to fix join_regimen() we'd need to also update new_regimen() so that rate is included for infusions. I can do this, but I wonder if it would instead make more sense to remove rate from non-infusion regimens instead? It seems a bit odd that we are currently including it only in regimens where it is not relevant.

@jasmineirx
Copy link
Contributor

When we simulate using the regimens, if there are non-zero infusion rates we calculate rate in create_event_table and otherwise we set rate to zero

https://github.com/InsightRX/PKPDsim/blob/master/R/create_event_table.R#L120-L131

so it's not clear to me how we are using rate -- possibly in other checks or times we extract info from regimens. I am ok with merging for now and tackling separately making rate work more consistently.

@jasmineirx jasmineirx self-requested a review January 8, 2024 20:41
@karawoo
Copy link
Contributor Author

karawoo commented Jan 8, 2024

Sounds good, I added a separate ticket to address the rate issue more generally

@karawoo karawoo merged commit b892eca into master Jan 8, 2024
2 checks passed
@karawoo karawoo deleted the RXR-1831 branch January 8, 2024 20:54
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.

2 participants