PyRenew Demo Questions #83

damonbayer · 2024-04-15T20:53:55Z

This draft PR is an unorganized collection of questions and ideas I had working through the most recent changes to pyrenew_demo.qmd (#77).

I hope that veteran contributors will respond to my comments, and we can eventually arrive at some concrete changes to the demo that will improve the experience for newcomers.

codecov · 2024-04-15T20:57:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.49%. Comparing base (437016d) to head (b9560be).

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #83   +/-   ##
=======================================
  Coverage   90.49%   90.49%           
=======================================
  Files          28       28           
  Lines         547      547           
=======================================
  Hits          495      495           
  Misses         52       52

Flag	Coverage Δ
unittests	`90.49% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gvegayon · 2024-04-15T21:48:58Z

model/docs/pyrenew_demo.qmd

@@ -49,8 +49,13 @@ with seed(rng_seed=np.random.randint(0,1000)):
    q_samp = q.sample(duration=100)

 plt.plot(np.exp(q_samp[0]))


Good question, we need to document what is the output of this function. Every sample function from pyrenew returns a tuple or named tuple.

gvegayon · 2024-04-15T21:49:38Z

model/docs/pyrenew_demo.qmd

 ```

+Damon: I believe the next section is totally separate from this first example. Perhaps we could make this clearer with # Section Labels.


gvegayon · 2024-04-15T21:50:49Z

model/docs/pyrenew_demo.qmd

@@ -103,6 +108,8 @@ inf_hosp_int = DeterministicPMF(
    (jnp.array([0, 0, 0,0,0,0,0,0,0,0,0,0,0, 0.25, 0.5, 0.1, 0.1, 0.05]),),
    )

+# These sum to 1, so I assume they are probabilities, but what is the domain of the distribution?
+# Regardless, a single array doesn't seem like the appropriate data structure to use.


Note that this is for a deterministic quantity that can be replaced with a probabilistic one. In the example, we assume the generation interval, but it can also be fitted.

I'm actually more confused now 😅

Two issues:

The doc mentions 18 possible outcomes and corresponding probabilities given by the values in the array. I understand that the array presents 18 probabilities. What values do those probabilities map to? Why are so many of them 0 instead of being omitted as possibilities?

I'm not sure what is meant to be communicated by "deterministic PMF." The docstrings mention a degenerate random variable. If that's the case, I don't understand why there are probabilities involved at all. Based on your description, I think it means that the prior and posterior distribution of this random variable are exactly the same. If that is the case, I think we could come up with a better descriptor. Something like fixed, constant, invariant, unestimated, or known

gvegayon · 2024-04-15T21:51:38Z

model/docs/pyrenew_demo.qmd

@@ -112,10 +119,12 @@ latent_hospitalizations = HospitalAdmissions(

 # 5) An observation process for the hospitalizations
 observed_hospitalizations = PoissonObservation()
+# Damon: What does it mean that there is a PoissonObservation? What are the parameters of the Poisson distrubtion?


Good point, the pyrenew_demo.qmd needs to include a model description as initial section.

gvegayon · 2024-04-15T21:53:03Z

model/docs/pyrenew_demo.qmd

@@ -130,6 +139,8 @@ hospmodel = HospitalizationsModel(
    latent_infections=latent_infections,
    Rt_process=Rt_process
    )
+# Damon: I don't really get why there is a hospitalizations model as a concept.
+# Damon: Maybe the scope of the project is so limited that it makes sense, but one can easily imagine additional data sources (Wastewater, separate hosp and ICU admissions). Would each of those get a separate class?


Indeed, that's the whole point. The current state features a handful of models and classes, but needs to be extended. Most of the required features (including other data models) are listed under the repo's issues.

gvegayon · 2024-04-15T21:53:40Z

model/docs/pyrenew_demo.qmd

@@ -138,6 +149,7 @@ Next, we sample from the `hospmodel` for 30 time steps and view the output of a
 with seed(rng_seed=np.random.randint(1, 60)):
    x = hospmodel.sample(n_timepoints=30)
 x
+# Damon: Why do we generate random number to use as the rng seed?


Good question, @dylanhmorris should know as he wrote the initial code!

gvegayon · 2024-04-15T21:53:50Z

model/docs/pyrenew_demo.qmd

@@ -152,9 +164,12 @@ ax[1].plot(x.latent)
 ax[2].plot(x.sampled, 'o')
 for axis in ax[:-1]:
    axis.set_yscale("log")
+# Damon: We should label the figures.


gvegayon · 2024-04-15T21:54:28Z

model/docs/pyrenew_demo.qmd

 ```

 To fit the `hospmodel` to the simulated data, we call `hospmodel.run()`, an MCMC algorithm, with the arguments generated in `hospmodel` object, using 1000 warmup stepts and 1000 samples to draw from the posterior distribution of the model parameters. The model is run for `len(x.sampled)-1` time steps with the seed set by `jax.random.PRNGKey()`
+Damon: Which MCMC algorithm is run? Does it come from another module?


NUTS, this should also be explained explicitly (or at least point to where there may be more details).

gvegayon · 2024-04-15T21:55:08Z

model/docs/pyrenew_demo.qmd

 ```

 To fit the `hospmodel` to the simulated data, we call `hospmodel.run()`, an MCMC algorithm, with the arguments generated in `hospmodel` object, using 1000 warmup stepts and 1000 samples to draw from the posterior distribution of the model parameters. The model is run for `len(x.sampled)-1` time steps with the seed set by `jax.random.PRNGKey()`
+Damon: Which MCMC algorithm is run? Does it come from another module?
+Damon: Where did we specify the prior distribution?


Nowhere explicitly, that's the beauty of numpyro. Priors are embedded in the sample() functions.

gvegayon · 2024-04-15T21:56:01Z

model/docs/pyrenew_demo.qmd

@@ -166,6 +181,8 @@ hospmodel.run(
    rng_key=jax.random.PRNGKey(54),
    mcmc_args=dict(progress_bar=False),
    )
+
+# Damon: What is the relationship between `n_timepoints` and `observed_hospitalizations`? Is `n_timepoints` always one less than the number of hospitalization observtion times? If so, is this parameter redundant?


Great points, that's something that needs to be addressed. When observed hospitalizations are passed, n_timepoints should be directly computed.

gvegayon · 2024-04-15T21:56:39Z

model/docs/pyrenew_demo.qmd

@@ -199,4 +218,6 @@ for samp_id in samp_ids:
 ax.set_ylim([0.4, 1/.4])
 ax.set_yticks([0.5, 1, 2])
 ax.set_yscale("log")
+
+# Can we use ArviZ for visualization? It is included in poetry dependencies.


What's that? If you think it improves things, for sure!

https://python.arviz.org/en/stable/index.html

It is included here, so it seems that someone else may have been keen to use it.

gvegayon · 2024-05-06T17:39:09Z

Hey @cshelley, here is the PR I was talking about. It'd be great if you could go through the questions/points @damonbayer raised and create a new issue listing them. Thanks!

damonbayer · 2024-06-14T19:38:05Z

Closing in favor of #191

damonbayer added 2 commits April 15, 2024 15:45

First pass at questions

c593418

cleanup

b9560be

damonbayer requested a review from gvegayon April 15, 2024 20:53

gvegayon reviewed Apr 15, 2024

View reviewed changes

gvegayon assigned cshelley May 6, 2024

damonbayer mentioned this pull request Jun 10, 2024

Identify areas for improvement in tutorials and tests #166

Closed

3 tasks

damonbayer closed this Jun 14, 2024

damonbayer deleted the dmb_pyrenew_demo_questions.qmd branch September 12, 2024 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PyRenew Demo Questions #83

PyRenew Demo Questions #83

damonbayer commented Apr 15, 2024 •

edited

Loading

codecov bot commented Apr 15, 2024 •

edited

Loading

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

damonbayer Apr 16, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

gvegayon Apr 15, 2024

damonbayer Apr 16, 2024

gvegayon commented May 6, 2024

damonbayer commented Jun 14, 2024

		@@ -49,8 +49,13 @@ with seed(rng_seed=np.random.randint(0,1000)):
		q_samp = q.sample(duration=100)

		plt.plot(np.exp(q_samp[0]))

		```

		Damon: I believe the next section is totally separate from this first example. Perhaps we could make this clearer with # Section Labels.

PyRenew Demo Questions #83

PyRenew Demo Questions #83

Conversation

damonbayer commented Apr 15, 2024 • edited Loading

codecov bot commented Apr 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvegayon commented May 6, 2024

damonbayer commented Jun 14, 2024

damonbayer commented Apr 15, 2024 •

edited

Loading

codecov bot commented Apr 15, 2024 •

edited

Loading