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

Experimental parallelisation of Ebola model #214

Closed
wants to merge 5 commits into from

Conversation

pratikunterwegs
Copy link
Collaborator

This draft PR is an experiment in parallelising the Ebola model using the {furrr} package, and is mostly aimed at checking whether this offers any performance gains. Changes:

  1. Using furrr::future_map() instead of Map() in .model_ebola_internal();
  2. Using {furrr} functionality for parallelisation-safe RNG seeds instead of `withr::local_seed();
  3. Setting up a multi-core future resolution plan for continuous benchmarking for futurised version of Ebola model.

Copy link

This pull request:

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

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

Copy link

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 21ebc61 is merged into main:

  • 🚀default_ode: 14.5ms -> 10.8ms [-29.9%, -21.69%]
  • ❗🐌default_ode_interventions: 71.4ms -> 99.5ms [+38.77%, +40.11%]
  • ✔️default_ode_param_vec: 827ms -> 818ms [-2.11%, +0.03%]
  • 🚀default_ode_paramvec_intervs: 6.24s -> 6.16s [-1.84%, -0.52%]
  • 🚀ebola: 599ms -> 462ms [-23.38%, -22.32%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@pratikunterwegs
Copy link
Collaborator Author

Seeing some dubious results on the benchmarking here as the parallelisations appears to speed up the ebola model but the PR also shows performance losses for the ODE model which wasn't touched in this PR. Running the CB again and closing this PR as a useful trial if checks remain unreliable.

Copy link

This is how benchmark results would change (along with a 95% confidence interval in relative change) if a6e001e is merged into main:

  • ✔️default_ode: 10.9ms -> 10.9ms [-1.96%, +1.45%]
  • 🚀default_ode_interventions: 99.5ms -> 72.5ms [-27.76%, -26.4%]
  • ✔️default_ode_param_vec: 833ms -> 835ms [-0.21%, +0.72%]
  • ✔️default_ode_paramvec_intervs: 6.26s -> 6.27s [-0.43%, +0.7%]
  • 🚀ebola: 561ms -> 473ms [-17.47%, -13.81%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@pratikunterwegs
Copy link
Collaborator Author

Closing this PR as results remain potentially unreliable, but it might be worth exploring parallelisation again in future.

@pratikunterwegs pratikunterwegs deleted the parallel-ebola branch June 10, 2024 14:17
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