Refactor distributions #112
Conversation
stisim/people.py
Outdated
| self.states = ss.ndict(base_states, states) | ||
| states = [ | ||
| ss.State('age', float), | ||
| ss.State('female', bool, False), |
There was a problem hiding this comment.
Replace with distribution -- if i'm following, this would be ss.State('female', bool, ss.choice([True,False])) right? Optionally with an argument for sex_ratio I guess.
I wonder if we'll need to be careful about dtypes here. Actully maybe this is an argument against #108. Or on the other hand, maybe not - maybe the distribution class could handle the dtypes without too much trouble.
There was a problem hiding this comment.
Yep just pushed that which will be a useful example to have in the codebase. I didn't add in the optional argument to set the sex ratio (although that's just an extra argument to choice) because I'm not sure how exactly we'd want to incorporate it.
Something I also realized in working on this refactor/PR is that this opens up the possibility of having the distribution function be based on data, e.g., it would be possible to define a Distribution that takes in binned age counts for a setting and then returns sampled ages accordingly. So that could give some new options for incorporating data too
There was a problem hiding this comment.
Thanks, I like how this PR gets around some of the arbitrariness of the implementation of sample(). A few comments/questions:
- I think it would be nice to support plain python objects where possible, so e.g. parameters can be exported to JSON and then imported. Could we have something like
InterventionDictbut for distributions? e.g.dist = dict(dist='poisson', rate=0.01); sti.dist(dist)would be equivalent tosti.poisson(rate=0.01)? - I can imagine users might be confused about when to use these distributions and when to use equivalent NumPy or SciPy distributions. Should we support SciPy distributions as well (which to me look more or less equivalent except with
sample()instead ofrvs). - Some of these distributions were introduced to support specific use cases for Covasim (and/or because this was the only way of providing a distribution), I wonder if we need all of them? That said, I agree we wouldn't want users to be forced to specify parameters as e.g.
partners = lambda n: np.random.poisson(0.01, size=n)instead ofpartners = sti.poisson(0.01).
|
Issues here: #120 |
Refactor sampling relating to #110