-
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
Refactoring sample from dicts to named arguments + **kwargs #63
Refactoring sample from dicts to named arguments + **kwargs #63
Conversation
Want me to review this? So 47 is not bottlenecked? |
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.
Some tweaks.
RE: Observation
classes: as discussed in person, we should standardize the name of the predicted value and observed value keyword arguments to an Observation
's sample()
method. I think obs
for observed (to be consistent with numpyro.sample()
. For the prediction, how about pred
, which is nicely consistent with obs
?
GIven this, I agree that more templating makes sense. Time for an abstract Observation(RandomVariable)
metaclass?
@AFg6K7h4fhy2 and @dylanhmorris, all done with your requests:
|
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.
Approve conditional on some minor comments being addressed.
No big changes since last check. Most unaddressed issues will be resolved with existing issues+PRs.
This PR includes the following:
RandomVariable
andModel
to use**kwargs
forsample()
.model
module, it ensures that arguments are via keywords instead of dictionaries.For a new discussion: Now that I've done this refactoring, the need to standardize the submodules became apparent.
RandomVariable.sample()
method ofobserved
class featuremean
andobs
. This way theHospitalizationModel
would always expect the observation process to receivemean
for the latent hospitalizations andobs
for the observed hospitalizations.**kwargs
are unpacked whenever calling aRandomVariable.sample()
method.observed
,process
could also have defaults forRandomVariable.sample()
, e.g.,n_timesteps
.