Skip to content

Mem 787 tutorial model run#812

Merged
dylanhmorris merged 45 commits into
mainfrom
mem_787_tutorial_model_run
May 13, 2026
Merged

Mem 787 tutorial model run#812
dylanhmorris merged 45 commits into
mainfrom
mem_787_tutorial_model_run

Conversation

@cdc-mitzimorris
Copy link
Copy Markdown
Collaborator

Revised tutorial Building Multisignal Models tutorial 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().

@cdc-mitzimorris cdc-mitzimorris requested a review from sbidari May 7, 2026 16:17
@cdc-mitzimorris
Copy link
Copy Markdown
Collaborator Author

@sbidari - this PR is just changing the tutorial on model building - your fresh eyes would be much appreciated!

@sbidari
Copy link
Copy Markdown
Contributor

sbidari commented May 7, 2026

Thanks for the flag @cdc-mitzimorris, I will review shortly!

Copy link
Copy Markdown
Contributor

@sbidari sbidari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread docs/tutorials/building_multisignal_models.qmd
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd
@cdc-mitzimorris
Copy link
Copy Markdown
Collaborator Author

@sbidari - ready for re-review.

@cdc-mitzimorris cdc-mitzimorris requested a review from sbidari May 11, 2026 17:34
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd Outdated
Comment thread docs/tutorials/building_multisignal_models.qmd
Copy link
Copy Markdown
Collaborator

@dylanhmorris dylanhmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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().

@cdc-mitzimorris
Copy link
Copy Markdown
Collaborator Author

all changes made.

@cdc-mitzimorris cdc-mitzimorris requested a review from sbidari May 12, 2026 21:26
Copy link
Copy Markdown
Collaborator

@dylanhmorris dylanhmorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @cdc-mitzimorris!

@dylanhmorris dylanhmorris merged commit 0f223f5 into main May 13, 2026
8 checks passed
@dylanhmorris dylanhmorris deleted the mem_787_tutorial_model_run branch May 13, 2026 18:08
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