-
Notifications
You must be signed in to change notification settings - Fork 14
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
Vivax competing hazards main drugs #330
base: dev
Are you sure you want to change the base?
Conversation
#' @export | ||
DHA_PQP_params <- c(.95, 0.09434, 4.4, 28.1) | ||
|
||
#' @title Preset parameters for the AL drug | ||
#' @description From SI of Commun. 5:5606 doi: 10.1038/ncomms6606 (2014) | ||
#' @details Use a vector of preset parameters for the AL drug (artemether-lumefantrine) | ||
#' @details Default parameters, from L to R, are: drug_efficacy: 0.95, drug_rel_c: 0.05094, drug_prophylaxis_shape: 11.3, drug_prophylaxis_scale: 10.6 | ||
#' @export | ||
AL_params_falciparum <- c(.95, 0.05094, 11.3, 10.6) | ||
|
||
#' @export | ||
AL_params <- c(.95, 0.05094, 11.3, 10.6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to keep the non-species specific drugs? Backward compatability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would lean towards keeping AL_params and removing AL_params_falciparum. Bumping the version and getting everyone to change their code is not convenient
R/human_infection.R
Outdated
@@ -370,9 +370,28 @@ relapse_bite_infection_hazard_resolution <- function( | |||
# get bite infections | |||
bite_infections <- infected_humans$copy()$and(relapse_infections$not(inplace = F)) | |||
|
|||
## drug prophylaxis may limit formation of new hypnozoite batches | |||
ls_prophylaxis <- rep(0, bite_infections$size()) | |||
if(length(parameters$drug_hypnozoite_efficacy)>0){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finding this section a little hard to follow.
In set_drugs()
or set_clinical_treatment()
I wasn't able to see any checks that if species == vivax, all drugs have the additional hypnozoite elements. If it isn't in there it might be good to have a check. What happens if these are NA in the simulation?
For example if I set drugs as:
drugs <- list(
malariasimulation::AL_params,
malariasimulation::CQ_PQ_params_vivax
)
In my parameters I get parameters$drug_hypnozoite_efficacy = NA 0.713, So
length(parameters$drug_hypnozoite_efficacy)>0is
TRUEhere. I think this is getting dealt with in line 379
ls_medicated[ls_drug > 0] <- !is.na(parameters$drug_hypnozoite_efficacy[ls_drug])`, but I wonder if it should be checked and raised as an issue to the user when setting parameters. That would be more transparent?
R/human_infection.R
Outdated
## all bitten humans with an infectious bite (incorporating prophylaxis) get a new batch of hypnozoites | ||
if(bite_infections$size()>0){ | ||
new_hypnozoite_batch_formed <- bite_infections # bitset_at(bite_infections, bernoulli_multi_p(1-ls_prophylaxis)) | ||
new_hypnozoite_batch_formed <- bitset_at(bite_infections, bernoulli_multi_p(1-ls_prophylaxis)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new_hypnozoite_batch_formed <- bitset_at(bite_infections, bernoulli_multi_p(1-ls_prophylaxis)) | |
new_hypnozoite_batch_formed <- bitset_at(bite_infections, bernoulli_multi_p(1 - ls_prophylaxis)) |
R/mda_processes.R
Outdated
@@ -76,6 +79,19 @@ create_mda_listeners <- function( | |||
variables$drug$queue_update(drug, to_move) | |||
variables$drug_time$queue_update(timestep, to_move) | |||
} | |||
|
|||
# Update liver stage drug effects | |||
if(!is.na(parameters$drug_hypnozoite_efficacy[drug])){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above. For clarity should all vivax drugs have this parameter (even if by default the user sets it to 0)?
b807634
to
16cc8c4
Compare
a34e336
to
bbb1ad4
Compare
16cc8c4
to
a75a671
Compare
bbb1ad4
to
f5a50c4
Compare
a75a671
to
2e0e3f3
Compare
f5a50c4
to
d5fa60e
Compare
2e0e3f3
to
6309897
Compare
d5fa60e
to
0582fec
Compare
921c84f
to
16666ba
Compare
…ers. Default drug parameters have been relabelled using parasite species names, although the original names have been left in for now, so that it can still be compatable with previous code. All the tests and vignettes use the parasite named parameters sets. Treatment, MDA and PMC code now includes hypnozoite clearance when these drugs are used. Added drug prophylaxis effects to human infection, preventing new hypnozoite batch formation.
0582fec
to
a24ebdd
Compare
/benchmark |
This is how benchmark results would change (along with a 95% confidence interval in relative change) if a24ebdd is merged into dev:
Further explanation regarding interpretation and methodology can be found in the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @RJSheppard.
A few Qs:
- I'm still a bit unclear as to why we have, for exampe both
SP_AQ_params_falciparum
andSP_AQ_params
. When is the second used? - I see we can use vivax drugs for MDA. What have we done with SMC and PMC? To my knowledge those aren't avaiable for vivax-specific implementation. Is it worth putting in a check to error if users try to set up either of those interventions with vviax drugs? I can't remeber but perhaps we already have a check to stop those interventions completely if
parasite = vivax
? - Worth sneeking in a fix for inconsistency in set_drug vector lengths does not introduce error #339 here?
@@ -729,6 +749,15 @@ calculate_treated <- function( | |||
) | |||
} | |||
|
|||
# Update liver stage drug effects | |||
if(length(parameters$drug_hynozoite_efficacy) > 0){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are both if
s required here?
If successfully_treated$successfully_treated_hypnozoites$size() > 0
wouldn't length(parameters$drug_hynozoite_efficacy) > 0
by definition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I've coded it means that successfully_treated$successfully_treated_hypnozoites
is only generated when length(parameters$drug_hynozoite_efficacy) > 0
. So it doesn't exist for falciparum, or for vivax when non hypnozoite drugs are being used (e.g., just CQ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be avoidable if all vivax drugs have that parameter (see other comment)?
#' @details Default parameters, from L to R, are: drug_efficacy: 1, drug_rel_c: 0.5, drug_prophylaxis_shape: 5, drug_prophylaxis_scale: 20, | ||
#' drug_hypnozoite_efficacy: 0.713, drug_hypnozoite_prophylaxis_shape: 5.5, drug_hypnozoite_prophylaxis_scale: 30 | ||
CQ_TQ_params_vivax <- c(1, 0.5, 5, 20, 0.713, 5, 30) | ||
|
||
#' @title Parameterise drugs to use in the model | ||
#' | ||
#' @param parameters the model parameters | ||
#' @param drugs a list of drug parameters, can be set using presets | ||
#' @export | ||
set_drugs <- function(parameters, drugs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any validation we could do here to validate if parameters$parasite == vivax that the correct drugs are used? Or do all drugs make sense for all parasites?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spoke with Pete about this a while ago, and we came to a place of hoping that people would use their good sense to choose which parameter sets to apply for each species. While drug parameters are parasite-specific, there might be circumstances where someone would want to see what happens if the same drug is applied to both species. I've added a few warnings to flag where people might do something strange while maintaining flexibility. I could change some of the warnings to errors (e.g., the length of the parameter inputs) if we think that's important.
R/human_infection.R
Outdated
|
||
ls_drug <- variables$ls_drug$get_values(bite_infections) | ||
ls_medicated <- (ls_drug > 0) | ||
ls_medicated[ls_drug > 0] <- !is.na(parameters$drug_hypnozoite_efficacy[ls_drug]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect this to fail? Since length(ls_medicated[ls_drug > 0])
is less than or equal to length(ls_drug)
. If some people are not medicated then there'll be a weird assignment here. Is this tested for?
ls_medicated[ls_drug > 0] <- !is.na(parameters$drug_hypnozoite_efficacy[ls_drug]) | |
ls_medicated <- ls_medicated & !is.na(parameters$drug_hypnozoite_efficacy[ls_drug]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you're saying. An issue is that some of ls_drug will be 0 (if no drugs have been assigned to the individual), which means that !is.na(parameters$drug_hypnozoite_efficacy[ls_drug])
may not have the same length as ls_medicated
(as far as I can work it out). I've taken a look to make sure it's doing what it's supposed to do, with a few tests to check it through.
#' @export | ||
DHA_PQP_params <- c(.95, 0.09434, 4.4, 28.1) | ||
|
||
#' @title Preset parameters for the AL drug | ||
#' @description From SI of Commun. 5:5606 doi: 10.1038/ncomms6606 (2014) | ||
#' @details Use a vector of preset parameters for the AL drug (artemether-lumefantrine) | ||
#' @details Default parameters, from L to R, are: drug_efficacy: 0.95, drug_rel_c: 0.05094, drug_prophylaxis_shape: 11.3, drug_prophylaxis_scale: 10.6 | ||
#' @export | ||
AL_params_falciparum <- c(.95, 0.05094, 11.3, 10.6) | ||
|
||
#' @export | ||
AL_params <- c(.95, 0.05094, 11.3, 10.6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would lean towards keeping AL_params and removing AL_params_falciparum. Bumping the version and getting everyone to change their code is not convenient
I've removed the vivax drugs can currently be used in SMC/PMC via the update_mass_drug_admin function that is primarily called during MDA (see mda_processes.R). It may not be realistic for this to be used in those settings, but for consistency/flexibility I've left it in. I can imagine circumstances where it might be interesting to look at TQ prophylaxis as SMC/PMC, given it's longer half-life. I can take a look at the drug vector length issue, but the example code is only causing a problem because the set_drugs function hasn't been used. The parameters are changed manually in the example, which isn't advised. I've added some warnings in set_drugs for when the vector length doesn't match up (and could make these errors if we want to fully restrict people from adding incorrect drug vector lengths). |
… Warnings added to set_drugs function for correct parasite drug parameter attribution. Tests to make sure liver stage prophylaxis is correct.
…vignette to reference drug impacts and prophylaxis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good thanks!
I've commented in line on a potentially redundant line, let me know if I'm mistaken.
R/human_infection.R
Outdated
|
||
ls_drug <- variables$ls_drug$get_values(bite_infections) | ||
ls_medicated <- ls_drug > 0 | ||
ls_medicated[ls_medicated] <- !is.na(parameters$drug_hypnozoite_efficacy[ls_drug]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite confusing. And possibly redundant? Don't we expect all liver stage drugs have a hypnozoite efficacy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the question, the way I've coded it is that the parameter sets are parasite specific, rather than being blood or liver stage specific. That means that vivax drug parameter sets can be either only blood stage (where they would have 4 parameters), or would have both blood and liver stage parameters (which total 7 parameters), and not all "vivax drugs" have liver stage action. Does that answer the question? (Happy to change things if you have a better way of doing things!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be more consistent to have liver-stage parameters for each vivax drug, and just set them to 0 if the drug is assumed to have no liver-stage impact?
…ponding aspect of vivax drug test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @RJSheppard - just a few minor followups from me
R/human_infection.R
Outdated
|
||
ls_drug <- variables$ls_drug$get_values(bite_infections) | ||
ls_medicated <- ls_drug > 0 | ||
ls_medicated[ls_medicated] <- !is.na(parameters$drug_hypnozoite_efficacy[ls_drug]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be more consistent to have liver-stage parameters for each vivax drug, and just set them to 0 if the drug is assumed to have no liver-stage impact?
@@ -729,6 +749,15 @@ calculate_treated <- function( | |||
) | |||
} | |||
|
|||
# Update liver stage drug effects | |||
if(length(parameters$drug_hynozoite_efficacy) > 0){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be avoidable if all vivax drugs have that parameter (see other comment)?
Adding drug vivax parameters, including liver stage prophylaxis.