ImperialCollegeLondon / covid19model

Code for modelling estimated deaths and cases for COVID19.
MIT License
944 stars 271 forks source link

Refactor stan according to issue #33 #38

Closed harrisonzhu508 closed 4 years ago

harrisonzhu508 commented 4 years ago

Implemented the code improvements suggested by Bob Carpenter in #33.

Problems

so I temporarily used the original loops as a placeholder (so that it works when tested on ubuntu and docker)

Thanks again to Bob Carpenter for the helpful comments

bob-carpenter commented 4 years ago

Which function isn't working?

If you can figure out a reusable function for the convolution, that'd be great. I couldn't see it easily because the use wasn't quite parallel in the two instances.

why does the centred parameterisation work better?

The centered parameterization will work better when there's enough data to make the posterior roughly normal (it's not just quantity of data, but how low variance it is and how consistent it is with the prior). Before that, the non-centered parameterization works better. The reason is that the non-centered parameterization dissociates the hierarchical and lower level parameters in the prior. There's a discussion in the user's guide around Neal's funnel example.

P.S. I'd also strongly suggest using spaces arond operators like +, -, = and adding space between ){. It maks the code easier to read when symbols aren't all jammed up.

harrisonzhu508 commented 4 years ago

Thanks for the prompt reply - @bob-carpenter - the expected_deaths function. I've commented it out for the moment in the stan file. The deaths calculation looks a bit messy right now because I was thinking of debugging the function first and using that instead.

Okay I see - will check out the funnel example!

And yes I've tried my best to add in the spaces - let me know if there's anything that doesn't look right! :smile:

s-mishra commented 4 years ago

@harrisonzhu508 is it ok if we close this? We already got faster and optimzed version from @MansMeg!

harrisonzhu508 commented 4 years ago

Sure, I'll close this :)