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

Incorporate cadre allocation computations more into mainline #259

Merged
merged 59 commits into from
Jun 3, 2023

Conversation

celiot-IDM
Copy link
Collaborator

@celiot-IDM celiot-IDM commented May 21, 2023

Phew ... this work started out with a small idea and turned into something very big.

The core work takes the cadre allocation computations that used to be done by functions in pace_post_process.R and moves them more into the mainline of process. So instead of using suite results CSV files as inputs, post-processing information can now be done immediately after an experiment finishes, based on the results structure returned by RunExperiments().

NOTE: THIS IS A BREAKING CHANGE. Old versions of the post-processing code won't work with the new Cadre allocation sheet format. A side-effect of making cadre roles user-configurable at the scenario/delivery-model level is that we can't naively combine suite result CSV files the way it was done before by ReadAndCollateSuiteResults(). (The key problem is that a user could configure a member ID - e.g "HEW1" - to mean one thing in one delivery model, but something different in another delivery model. Different delivery models have different cadre memberships - MergedModel = {HEW1 | HEW2 | FHP1 | FHP2}, but BasicModel = {HEW1 | HEW2 | FHP1 | FHP2 | RN | EHP} - so it's possible that in trying to create one big view that crosses several delivery models, we could end up aggregating things that shouldn't be aggregated.)

Annual overheads are calculated for all cadre members in all models, but I'm not sure how you want to access that information.

Here's a sample of the new approach.

library(pacehrh)
pacehrh::Trace(TRUE)

pacehrh::InitializePopulation()
pacehrh::InitializeScenarios()
pacehrh::InitializeStochasticParameters()
pacehrh::InitializeSeasonality()
pacehrh::InitializeCadreRoles()

pacehrh::SetGlobalStartEndYears(2020, 2050)
pacehrh::SetRoundingLaw("none")

results <-
  pacehrh::RunExperiments(scenarioName = "MergedModel",
                          trials = 20)

SR <- pacehrh::SaveExtendedSuiteResults(results, run = "Run_ID")
CA <- pacehrh::SaveCadreAllocations(SR)

summaryStats <- pacehrh::ComputeSummaryStats(SR, CA)

Two additional bits of magic ...

Magic 1: If you don't need to keep the output from SaveExtendedSuiteResults() (basically a much wider version of the standard output, but extended with Scenario and Task information), you can speed things up a bit by piping:

# Create the cadre allocation table without saving intermediate results locally or to disk
CA <-
  pacehrh::SaveExtendedSuiteResults(results) %>%
  pacehrh::SaveCadreAllocations()

Magic 2: Both SaveExtendedSuiteResults() and SaveCadreAllocations() take filepath parameters which will save their outputs to disk.

# Create CA table and save both the extended suite results and the cadre allocation results
CA <-
  pacehrh::SaveExtendedSuiteResults(results, filepath = "_SR.csv", run = "Run-100") %>%
  pacehrh::SaveCadreAllocations(filepath = "_CA.csv")

I have some more tidying up to do, but I'm going to move on to finish off the stochastic rate capping work now.

Addresses #51

@celiot-IDM celiot-IDM added the enhancement New feature or request label May 21, 2023
@celiot-IDM celiot-IDM added this to the V1.1 - PACE-HRH refresh milestone May 21, 2023
@celiot-IDM celiot-IDM requested a review from MeWu-IDM May 21, 2023 19:36
@celiot-IDM
Copy link
Collaborator Author

celiot-IDM commented May 21, 2023

@MeWu-IDM : Integration tests are failing, but I don't understand why. Over to you!

Update: Working now after merging input format changes from 244 into 51.

@MeWu-IDM
Copy link
Collaborator

does pacehrh:::SaveCadreAllocations not work with the results from pacehrh::ReadAndCollateSuiteResults after this change?
I ran into error: more than one scenario in suite results

@rhanIDM
Copy link
Collaborator

rhanIDM commented Jun 1, 2023

@celiot-IDM, is RoleID required to be unique for task time allocation to cadre to compute correctly?

pacehrh/DESCRIPTION Outdated Show resolved Hide resolved
@celiot-IDM
Copy link
Collaborator Author

@celiot-IDM, is RoleID required to be unique for task time allocation to cadre to compute correctly?

The overhead times calculation will work just fine with duplicate RoleID values. After that it's up to you (the researcher) to collate RoleID's with the role information embedded in the headers of the cadre task sheets.

celiot-IDM and others added 6 commits June 2, 2023 13:53
Set rounding = "Late" to match Run_simulations
Delete: Run_simulations_new.R and model_inputs_shiny_demo.xlsx were used for testing purposes only.
Update: Run_simulations.R, model_inputs_demo.xlsx
Add: Analysis_pipeline_demo.R
Add dply:: to summarise, mutate and arrange
Copy link
Collaborator

@MeWu-IDM MeWu-IDM left a comment

Choose a reason for hiding this comment

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

We were able to update the scripts to produce the same cadre allocation results (Run_simulation.R is updated)
Regression tests are also updated to cover the new cadre calculation.

@github-actions
Copy link

github-actions bot commented Jun 3, 2023

Code Coverage

Package Line Rate Complexity Health
pacehrh 90% 0
Summary 90% (2445 / 2702) 0

Minimum allowed line rate is 60%

Copy link
Collaborator

@rhanIDM rhanIDM left a comment

Choose a reason for hiding this comment

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

Run_simulations.R is updated to post-process new cadre allocations results structure. Model_inputs_demo.xlsx is updated to new structure and we were able to use these inputs to confirm the calculation results match the old results.

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.

4 participants