In estimate_joint, fix tmin to be the 95th percentile of the SI distribution #127
Conversation
This file contains 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
To be conservative, I have chosen maximum across all variants. Also written a new test to check it works.
Updated vignette to use library rather than require
- Modified vignette to use library rather than require - explicitly set t_min to 2 in draw_R calls in tests because the default is set to NULL in estimate_joint - All tests for estimate_joint implicitly test cutoff as well.
annecori
reviewed
Jul 20, 2021
R/gibbs_draws.R
Outdated
| ##' @author Sangeeta Bhatia | ||
| ##' @export | ||
| compute_si_cutoff <- function(si_distr, miss_at_most = 0.05) { | ||
| if (sum(si_distr) != 1) { |
There was a problem hiding this comment.
this suggests that si_distr is a vector here i.e. for a single variant, so I don't think you can inheritParams and you need to write a help for the si_distr param?
There was a problem hiding this comment.
actually a couple of lines below you use colSums; so not sure which is the correct one but they don't seem consistent with one another?
annecori
reviewed
Jul 20, 2021
There was a problem hiding this comment.
Looks good but:
- I have a comment about the format of si_distr used in the new function to compute the quantile.
- I don't see a test for the new code?
- I wonder whether we should also add something to enforce t_min being after the first case of each variant, so that you can't start the estimation before the first case
t_min is now maximum day of non-zero incidence + max(95th percentile of SI distributions). Added tests for each bit.
I am specifying t_min as 2 in older tests now because they were set-up to work with that default.
annecori
approved these changes
Jul 20, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If t_min is NULL, it is set to be the maximum of the 95th percentile of the SI distributions across all variants.
Added a test to check that it works as expected with a reasonable SI distribution.
All tests pass ok