Mem 787 tutorial model run#812
Conversation
|
@sbidari - this PR is just changing the tutorial on model building - your fresh eyes would be much appreciated! |
|
Thanks for the flag @cdc-mitzimorris, I will review shortly! |
sbidari
left a comment
There was a problem hiding this comment.
This tutorial is comprehensive! A main comment I have is around the helper function for data prep. I would prefer inline data prep and model fit to only one dataset (either 90 days or 180 days). Other than that, looks great to me.
|
@sbidari - ready for re-review. |
dylanhmorris
left a comment
There was a problem hiding this comment.
Thanks @cdc-mitzimorris. Read through the rendered version and found some minor editing stuff that isn't actually in the diff, but I think can and should be addressed here.
One critical change requested: given that the motivation for this PR is to teach users to run models, I think we should remove the make_rng_key() function and explicitly create rng keys every time we call model.run().
|
all changes made. |
dylanhmorris
left a comment
There was a problem hiding this comment.
Thanks, @cdc-mitzimorris!
Revised tutorial
Building Multisignal Modelstutorial to address issue #787.The section on model fitting is now written from the user perspective. It explains how the observation data is aligned on the model time axis. It glosses over the details of the alignment process in order to keep the narrative flow on track, preserving only what's needed in order to follow the call to
model.run().