Skip to content
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

Rt with infection feedback #123

Merged
merged 47 commits into from
Jun 3, 2024
Merged

Rt with infection feedback #123

merged 47 commits into from
Jun 3, 2024

Conversation

gvegayon
Copy link
Member

@gvegayon gvegayon commented May 17, 2024

This PR:

  • Creates the InfectionsWithFeedback latent module.
  • Adds the new module pyrenew.datautils for common data management operations.
  • Adds the tutorial extending_pyrenew.qmd
  • Corrects the order of outputs from the sample_infections_with_feedback function.
  • Adds tests modules for InfectionsWithFeedback and pyrenew.datautils.

Extra:

  • Uses jax.numpy.atleast_1d() for DeterministicRV.sample() (so scalars are coerced into 1d arrays)
  • Uses jax.numpy.atleast_1d() for DistributionalRV.sample() (so scalars are coerced into 1d arrays)
  • Fixes the reminder of the ghost outbreak.

@gvegayon gvegayon linked an issue May 17, 2024 that may be closed by this pull request
4 tasks
Copy link

codecov bot commented May 17, 2024

Codecov Report

Attention: Patch coverage is 94.20290% with 4 lines in your changes missing coverage. Please review.

Project coverage is 93.90%. Comparing base (f9c057a) to head (6f70084).

Files Patch % Lines
model/src/pyrenew/latent/infectionswithfeedback.py 89.74% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
+ Coverage   92.06%   93.90%   +1.84%     
==========================================
  Files          34       36       +2     
  Lines         643      706      +63     
==========================================
+ Hits          592      663      +71     
+ Misses         51       43       -8     
Flag Coverage Δ
unittests 93.90% <94.20%> (+1.84%) ⬆️

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 gvegayon marked this pull request as ready for review May 21, 2024 17:53
@gvegayon gvegayon mentioned this pull request May 21, 2024
4 tasks
@gvegayon
Copy link
Member Author

gvegayon commented May 23, 2024

@dylanhmorris, 219e710 and 6db0fb7 fixed the failing tests. I had to introduce three changes:

  • Make sure the output from latent.Infections.sample() is at least 1 dimension (link).
  • Pad the sampled I0 within model.RtInfectionsRenewalModel.sample() to match the generation interval (link).
  • Still add some padding for the force of infection, otherwise jax.lax.scan breaks as it needs the xs argument needs the arrays to have the same shape (link).

This introduced a couple of other changes:

  • It seems it solved a bug in the "ghost outbreak." In the case of the infections function, before your changes, the conv scanner was using jnp.dot() to multiply an array with a scalar, which resulted in inflated infections at the beginning of the sequence. Compare these two images, this is what we have in main:

image

versus the PR

image

Because of this change, I had to update the reference figure generated during pytest. I'm creating an issue to include a note on the developers' documentation discussing the pytest involving matplotlib.

I think we are good to go!

model/pyproject.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@damonbayer damonbayer left a comment

Choose a reason for hiding this comment

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

One very minor change requested. I made the change myself.

@damonbayer damonbayer self-requested a review June 3, 2024 21:26
@damonbayer
Copy link
Collaborator

@dylanhmorris Needs your final review.

dylanhmorris
dylanhmorris previously approved these changes Jun 3, 2024
model/docs/extending_pyrenew.qmd Outdated Show resolved Hide resolved
@dylanhmorris dylanhmorris dismissed stale reviews from damonbayer and themself June 3, 2024 21:43

@damonbayer made the requested changes

@dylanhmorris
Copy link
Collaborator

Update: needs conflicts resolved @damonbayer. Can you do and request re-review from me?

Copy link
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.

LGTM

@dylanhmorris dylanhmorris merged commit f11ce38 into main Jun 3, 2024
8 checks passed
@dylanhmorris dylanhmorris deleted the 9-infection-feedback-in-rt branch June 3, 2024 22:20
damonbayer added a commit that referenced this pull request Jun 5, 2024
commit 879c665
Author: George G. Vega Yon <g.vegayon@gmail.com>
Date:   Wed Jun 5 11:29:09 2024 -0600

    Weekly Rt with autoregressive difference (#131)

    Co-authored-by: Damon Bayer <xum8@cdc.gov>
    Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>

commit f0a35ee
Author: George G. Vega Yon <g.vegayon@gmail.com>
Date:   Wed Jun 5 10:32:48 2024 -0600

    Renaming datautils to arrayutils (#154)

commit 394a03d
Author: Damon Bayer <xum8@cdc.gov>
Date:   Tue Jun 4 16:49:10 2024 -0500

    Record deterministic `RandomVariables` by default (#148)

commit a2c1307
Author: Damon Bayer <xum8@cdc.gov>
Date:   Tue Jun 4 16:02:07 2024 -0500

    read date columns as dates (#153)

commit f11ce38
Author: George G. Vega Yon <g.vegayon@gmail.com>
Date:   Mon Jun 3 16:20:08 2024 -0600

    Rt with infection feedback (#123)

    ---------

    Co-authored-by: Dylan H. Morris <dzl1@cdc.gov>
    Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
    Co-authored-by: Damon Bayer <xum8@cdc.gov>

commit f9c057a
Author: George G. Vega Yon <g.vegayon@gmail.com>
Date:   Fri May 31 13:18:58 2024 -0600

    Refactor transformation module to wrap `numpyro.distributions.transforms` (#140)

    Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
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.

Infection feedback in R(t)
4 participants