Skip to content

Add pressure testing functions#14

Merged
pratikunterwegs merged 30 commits into
mainfrom
develop
Jun 19, 2026
Merged

Add pressure testing functions#14
pratikunterwegs merged 30 commits into
mainfrom
develop

Conversation

@pratikunterwegs

@pratikunterwegs pratikunterwegs commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

This PR adds functions from the pressure testing report.

  • Added impact diagnostics functions in R/fn_impact_diagnostics.R.
  • Added plotting preparation functions and plotting functions in R/fn_plotting_prep_impact_diagnostics.R and R/fn_plotting_impact_diagnostics.R.
  • Added dependencies diffdf and here.
  • Added vignettes on pressure testing functions and package design.
  • Minor change renaming files and reorganising documentation to be clearer.

Getting started

The easiest way to get started with reviewing is to check out the develop branch and render the package website using {pkgdown}: pkgdown::build_site(). View the vignette on pressure testing under Articles which explains the added functions.

Open task

  • Figure out what data can be added to pkg to allow for testing
  • Test plotting preparation functions
  • Test plotting functions
  • Add vignette documentation with example data
  • Rename file re: impact diagnostics to pressure testing? (?)

Comment thread R/fn_pressure_testing.R Outdated
Comment thread R/constants.R Outdated
Comment thread R/fn_pressure_testing.R Outdated
Comment thread R/fn_pressure_testing.R Outdated
#' @export
gen_combined_df <- function(prev_dat, df2, interest_cols, key_cols) {
# TODO: input checks
# TODO: df2 needs a better name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

df2 is essentially a cleaned version of of the touchstone_new latest report on packit where vaccine names are cleaned, subregions appended etc. happy to take suggestions on a better way to do it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks - could you suggest an informative name for the argument that somebody new to the package and the workflow would understand? data_current? data_cleaned? If you could pop a couple of lines in this thread about any expectations around it, I can add those to the function docs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

df_clean is fine, thanks. df_clean is the imported dependency report (currently named touchstone_new) with the following cleaning steps applied:

  • Malaria vaccine names amended
  • Subregions appended
  • Fixes for missing country names
  • Removal of now-redundant columns
    It may even be sufficient to just apply these steps to df itself, I think I just kept them separate when developing the report so I didn't have to keep pulling a clean dependency when testing things out.

Comment thread R/fn_pressure_testing.R Outdated
@github-actions

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

Comment thread R/fn_plotting_impact_diagnostics.R Outdated
# for scales formatting
.x <- NULL

# TODO: should NA-producing values (< 1) be removed?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think previously we have used a lowerbound <- 1e-8 to filter out very small values

Comment thread R/fn_impact_diagnostics.R Outdated
) {
checkmate::assert_data_frame(df, min.cols = 1L, min.rows = 1L)

# TODO: can we find checks for prev_data size in reln to df? rows? cols?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure I fully understand? But prev_dat and df would likely be matched on columns

Comment thread R/fn_impact_diagnostics.R Outdated
variable <- rlang::arg_match(variable)
checkmate::assert_character(group_cols, min.len = 1L, any.missing = FALSE)

# TODO: check what a sensible upper limit might be

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think in my pressure testing report, an arbitrary limt was difference exceeding ± 100 times the interquartile range (IQR) for the relevant country, vaccine and activity_type.

Comment thread R/fn_impact_diagnostics.R

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To discuss on call whether to keep pressure testing functions and rapid-model-run-impact function separate or combine for ease

@pratikunterwegs pratikunterwegs self-assigned this Apr 29, 2026
@pratikunterwegs pratikunterwegs changed the title WIP adding pressure testing Add pressure testing functions Apr 29, 2026
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@pratikunterwegs pratikunterwegs added the enhancement New feature or request label Jun 5, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@pratikunterwegs pratikunterwegs marked this pull request as ready for review June 9, 2026 15:36
Comment thread R/fn_impact_diagnostics.R
)
}

# data may have a `burden_outcome` column which should not be counted as

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think burden_outcome should be moved to the default key_cols COLNAMES_KEY_PRESSURE_TEST; and be picked up in the pressure_testing.Rmd L98-99.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I looked into adding this to COLNAMES_KEY_PRESSURE_TEST, and this causes the problem that these names are used in functions where the burden is expected to be in wide format, so a separate column for each burden type. I think the parsimonious course of action is to add this to the checked columns here, but to leave it out from the constant vector for now. See example function filter_invalid_trajectories() below this one, where the outcomes are expected as "*_averted".

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see. Thanks for the clarification.

flag_duplicates(eg_impact) %>%
select(
modelling_group, country, disease,
vaccine, activity_type, year, n_key

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

as mentioned ealier, burden_outcome may also be picked up here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added burden_outcome here.

Comment thread vignettes/pressure_testing.Rmd Outdated
names_from = "burden_outcome",
values_from = "impact"
)
prev_df$support_type <- "other" # unsure what values this can take

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We are moving from support_type ("total", "gavi", "other") to percentage of gavi support (numeric between 0-1). And this will not be part of burden processing or impact calculation. It will only appear in our out-facing impact extrapolation reports for funders; and highly dependent on funders incremental updates (twice a year). I think we can ignore this classifier for the pressure testing.

@github-actions

Copy link
Copy Markdown

This pull request:

  • Adds 4 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@pratikunterwegs pratikunterwegs merged commit df208a4 into main Jun 19, 2026
11 checks passed
@pratikunterwegs pratikunterwegs deleted the develop branch June 19, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants