-
Notifications
You must be signed in to change notification settings - Fork 21
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
Feature: exponential delays #84
Conversation
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.
Looking good to me. Would it be worth reserving dist == 0
for a model option without parametric delay distribution at all, i.e. the nonparametric components only?
Minor point: should update the comment in l.33 of inst/stan/epinowcast.stan
.
Thanks for the review. Yeah, I considered this and decided against it initially. Reconsidering I think you are right, retooling this now and adding the ability to turn the parametric model on/off at the same time (though without putting anything else in so this will just break if accessed). Note bumped from Comment fixed in: 2f254b3 |
I've updated this so that As part of these changes I've done a little bit of internal refactoring to make things easier (@adrian-lison |
Codecov Report
@@ Coverage Diff @@
## develop #84 +/- ##
===========================================
- Coverage 55.65% 55.14% -0.51%
===========================================
Files 12 12
Lines 911 923 +12
===========================================
+ Hits 507 509 +2
- Misses 404 414 +10
Continue to review full report at Codecov.
|
Merging but we should circle back with a check before |
This PR closes #83 by adding support for exponential delays. It has no impact on default functionality. Checking the exponential delays posteriors they look reasonable. Checking posterior predictions indicates that performance is slightly worse than with the log-normal (at least on this very limited test set). The major issue seems to be capturing the first day of reporting which I find a little surprising.