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

refactor pipeline smaller scripts and functions #217

Merged
merged 24 commits into from
May 13, 2024

Conversation

SamuelBrand1
Copy link
Collaborator

@SamuelBrand1 SamuelBrand1 commented May 10, 2024

This PR aims to refactor, but not extend the functionality, of the analysis pipeline in such a way that:

  1. It is more readable.
  2. Its closer to DRY principles.
  3. Its written in such a way that a next step for job scheduling is as easy as possible.

Concretely,

  • Default variables are generated by calling a default function from the AnalysisPipeline module.
  • Vectors of scenarios combinations are generated by calling make_... functions from the AnalysisPipeline module.
  • Plotting functions are declared separately; I don't feel they belong inside the analysis module but open to changing this.
  • The pipeline is now one script, which essentially follows an analogous sequence of steps to the opening example of targets.R.
  • Inference scenarios are an inner loop of the the "truth data" generation process.
  • Each function has a unit test, as does the whole pipeline for a randomly selected inference scenario on a short flat time series of fake data.

Closes #215 .

@SamuelBrand1 SamuelBrand1 linked an issue May 10, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.73%. Comparing base (4262a9b) to head (9180cd0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #217   +/-   ##
=======================================
  Coverage   92.73%   92.73%           
=======================================
  Files          42       42           
  Lines         399      399           
=======================================
  Hits          370      370           
  Misses         29       29           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelBrand1 SamuelBrand1 requested a review from seabbs May 13, 2024 08:44
@seabbs seabbs enabled auto-merge (squash) May 13, 2024 09:15
Copy link
Collaborator

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

I really like this. I had a few suggestions to make it more complicated/have more infra but not essential. We can discuss and port what we want to do into new issues and merge this I think

pipeline/plots/plot_functions.jl Outdated Show resolved Hide resolved
pipeline/plots/plot_functions.jl Outdated Show resolved Hide resolved
pipeline/scripts/analysis_pipeline.jl Outdated Show resolved Hide resolved
pipeline/scripts/analysis_pipeline.jl Show resolved Hide resolved
pipeline/scripts/analysis_pipeline.jl Outdated Show resolved Hide resolved
pipeline/scripts/analysis_pipeline.jl Show resolved Hide resolved
pipeline/src/default_Rt.jl Show resolved Hide resolved
pipeline/src/default_priors.jl Outdated Show resolved Hide resolved
@SamuelBrand1
Copy link
Collaborator Author

SamuelBrand1 commented May 13, 2024

I've made a few minor changes:

  • More small(ish) functions with tests
  • plotting functions to inside AnalysisPipeline
  • moved some struct construction to inside a generate function to make the pipeline script a bit cleaner.

#218 Should cover some of the more exciting ideas we could do with pipeline analysis going forwards! But I think this pipeline (which runs) is currently a good template to layer on the rest the tasks in #195

@seabbs seabbs merged commit b65a810 into main May 13, 2024
10 checks passed
@seabbs seabbs deleted the 215-refactor-pipeline-smaller-scripts-and-functions branch May 13, 2024 13:51
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.

Refactor pipeline: smaller scripts and functions
3 participants