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

Changed saved prior predictive data #478

Conversation

SamuelBrand1
Copy link
Collaborator

This PR changes the serialisation behaviour of the pipeline to reduce unnecessary objects being serialised (less important) and deals with problems in deserialising around reconstructing arbitrary Julia types (more important).

Closes #477 .

NB: While doing this PR, I was passing more pipeline data to infer... so it became easier to just pass the pipeline struct instance. This lets us select the inference post-processing based on the pipeline (e.g. testmode, priorpredictive etc). There might be a slicker way to do this which we can revisit in the future.

@SamuelBrand1 SamuelBrand1 requested a review from seabbs October 7, 2024 14:00
Copy link
Contributor

github-actions bot commented Oct 7, 2024

Try this Pull Request!

Open Julia and type:

import Pkg
Pkg.activate(temp=true)
Pkg.add(url="https://github.com/CDCgov/Rt-without-renewal", rev="477-change-saved-prior-predictive-data", subdir="EpiAware")
using EpiAware

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.

Nice. I like the move to passing the pipeline type here vs the boolean.

@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (404-upgrade-the-testmode-pipeline-script-to-just-pipeline-script@ae8e3ca). Learn more about missing BASE report.

Additional details and impacted files
@@                                         Coverage Diff                                         @@
##             404-upgrade-the-testmode-pipeline-script-to-just-pipeline-script     #478   +/-   ##
===================================================================================================
  Coverage                                                                    ?   90.05%           
===================================================================================================
  Files                                                                       ?       57           
  Lines                                                                       ?      744           
  Branches                                                                    ?        0           
===================================================================================================
  Hits                                                                        ?      670           
  Misses                                                                      ?       74           
  Partials                                                                    ?        0           

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

…ne-script' into 477-change-saved-prior-predictive-data
@seabbs seabbs merged commit 234c77f into 404-upgrade-the-testmode-pipeline-script-to-just-pipeline-script Oct 7, 2024
3 checks passed
@seabbs seabbs deleted the 477-change-saved-prior-predictive-data branch October 7, 2024 15:02
github-merge-queue bot pushed a commit that referenced this pull request Oct 7, 2024
…dictive + full (#445)

* remove most compat from pipeline

* Update test_pipelinefunctions.jl

* Fix the truthdata output to be missing or Int

* change to daily increments in latent processes

* Change default AD mode to ReverseDiff{true}

* Create changelog.md

* reformat

* change filename

* Update simulate.jl

* add a testmode to pipelinetypes

* tighten typing

* fix constructor

* fix constructor

* pipeline helper functions

* unit tests and end-to-end tests

* remove old scripts

* Fix y_t type

* bring inference prefixing into line with truthdata prefixing

* reformat

* interim commit

* rename and collect plot functions

* tidy up tests

* basic plots to Makie

* new plot tests and test reorganisation

* Update runtests.jl

* reformat

* 455 plotting methods of prior predictive (#462)

* add priorpredictive method to make_inference

* Add pipeline priorpredictive boolean

* reformat

* remove inference_method kwarg because can be dispatched on

* remove specialisation on forecast results

add missing handling as well

* move inference step into own function

and give fail cover with error report

* remove passing inference_method

* add a latent model name to InferenceConfig

* Util for setting up PI levels

* prior pred plot

* export prior_predictive_plot

* remove dead end-to-end test

in favour of direct test of prior_predictive_plot

* Make pipeline inference tests more focused (#475)

* fix tests and capture forecasting failures

* delete stale end-to-end test

* Full priorpred check pipeline script (#476)

* Hotfix: Doc link to main

* create xexpy function with x = 1 typestable (#473)

* rename data folder

* catch bug where objects were getting saved to root dir

* fix bug where being saved to root

* add priorpredictive mode to the inference config struct

* do_truthdata behaviour modifed by prior predictive

* send priorpredictive mode in generate_inference

* update tests

* prior predictive pipeline

* reformat

---------

Co-authored-by: Sam Abbott <[email protected]>

* Add oneexpy to pipeline (#480)

* Changed saved prior predictive data (#478)

* change to pipeline being part of InferenceConfig

This also splits figure saving from data saving

* fix passed var

* change serialisation target dir

* update tests and config usage to pass pipeline object

---------

Co-authored-by: Sam Abbott <[email protected]>

* Update run_priorpred_pipeline.jl

* Only save strings rather than Exception objects

---------

Co-authored-by: Sam Abbott <[email protected]>
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.

3 participants