-
Notifications
You must be signed in to change notification settings - Fork 4
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
Issue 408: ARMA and ARIMA models #438
Conversation
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="issue408", subdir="EpiAware")
using EpiAware |
Hey do you need any help with this one? |
no its okay I just need to get off |
Seeing updates 👍 |
ha yes this PR has got really out of hand - just doing some lsthm admin stuff then back on this |
* Patch: Switch to fork of benchmarkCI (#520) * patch to fork of benchmarkCI * put fork version of BenchmarkCI in [sources] * swap order * add EpiAware [source] * fix path * rm benchmarkCI from project * Patch fix: add `Manifest.toml` to benchmarking (#524) * trigger * Update benchmark.yaml * Update benchmark.yaml * commit benchmark Manifest * try alternate approach * Update benchmark.yaml * Update EpiMethod.jl * Update benchmark.yaml * change baseline to origin/main * remove trigger * rm other trigger * Issue 465: Add an infection generating model for ODE problems (#510) * CompatHelper: bump compat for Turing to 0.35 for package EpiAware, (drop existing compat) (#516) * CompatHelper: bump compat for Turing to 0.35 for package EpiAware, (drop existing compat) * Update Project.toml * fix Project.toml --------- Co-authored-by: CompatHelper Julia <[email protected]> Co-authored-by: Sam Abbott <[email protected]> Co-authored-by: Samuel Brand <[email protected]> Co-authored-by: Samuel Brand <[email protected]> * CompatHelper: bump compat for DynamicPPL to 0.30 for package EpiAware, (drop existing compat) (#528) Co-authored-by: CompatHelper Julia <[email protected]> * rename IDD -> IID * rename test file * Issue 529: Create null Latent model (#530) * Null Latent model * Null Latent model * fix doctest * fix generate_epiaware unit tests New usage of RW * fix turing method test underlying std of step size changed name * fix broadcast test Underlying std param changed name * fix HN unit test Default std prior had changed * fix AR unit tests --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: CompatHelper Julia <[email protected]> Co-authored-by: Sam Abbott <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
I think a field name change in |
NB: I did quick fix of the prior ordering on top of the mishra note fix of @seabbs |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
I put in some control code and reduced the broadcasting in |
@SamuelBrand1 any chance you can get to this today? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core contribution of this PR is adding the MA
moving-average model, and helper functions that stack MA
AR
and DiffLatent
to construct any ARIMA model through model composition. I think this is a great pattern, and opens the way to using ARIMA models inside/compositionally with model units that are more specialised to epi modelling.
Also lot of changes to promote common approaches in the code base. This is really good.
My only blocker to merging is that I'm not sure that #440 is addressed, which was part of the plan (cf #441). I don't think thats a major lift (it might already be addressed but I'm not sure). Otherwise, just some comments that look like future issues.
This seems possible. I've spotted some places we could have tried a parameteric dispatch e.g. have a method where the |
So my understanding was that this PR blocked #441 but wasn't trying to address #440 (hence not doing so). If you agree I think we should widen the scope of #440 to all parameters of this kind ot make sure they are in the order people might expect. |
I'm happy with this sequence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR closes #407 and starts towards adding submodels everywhere (#372). To close this out I need to:
HierarchicalNormal
as the default residual processarma
andarima
helper functions. Deciding how much flexibility to expose (plan is to restrict to just p,q,r priors and the standard deviation prior of the residuals).