-
Notifications
You must be signed in to change notification settings - Fork 4
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
update infectionswithfeedback process #440
update infectionswithfeedback process #440
Conversation
…-handle-batch-input-within-numpyro-plate-context
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #440 +/- ##
==========================================
+ Coverage 94.03% 94.26% +0.23%
==========================================
Files 42 42
Lines 989 994 +5
==========================================
+ Hits 930 937 +7
+ Misses 59 57 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ate-infectionswithfeedback-process-to-handle-batch-input-within-numpyro-plate-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice @sbidari, but I think there are some opportunities to improve performance and clarity.
…-handle-batch-input-within-numpyro-plate-context
…ch-input-within-numpyro-plate-context' of https://github.com/CDCgov/PyRenew into 435-update-infectionswithfeedback-process-to-handle-batch-input-within-numpyro-plate-context
@sbidari Can we also have tests that try to use |
…-handle-batch-input-within-numpyro-plate-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few open questions. I would also like @dylanhmorris to take a look, if he can, since he just did similar work on the AR class.
…ch-input-within-numpyro-plate-context' of https://github.com/CDCgov/PyRenew into 435-update-infectionswithfeedback-process-to-handle-batch-input-within-numpyro-plate-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks generally very good, thanks @sbidari. A few questions about scope on which I will defer to you and @damonbayer, and a suggestion about dot products that will be useful for future-proofing.
Thanks @dylanhmorris |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A key remaining question about the dimensionality check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One final thing @sbidari, otherwise looks ready!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@damonbayer can you confirm your concerns were resolved? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sbidari!
This PR modifies the following functions/files such that these now work when
I0
andRt
are 2D arrays as is the case for site-level hierarchical model.new_convolve_scanner
andnew_double_convolve_scanner
pad_edges_to_match
that pads multidimensional arrays using speficied edge valuesInfections
andInfectionWithFeedback
classes in latent moduleInfection
andInfectionWithFeedback
classesOut of scope