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

Simplify compartmental model interface #2445

Merged
merged 2 commits into from
Apr 26, 2020
Merged

Simplify compartmental model interface #2445

merged 2 commits into from
Apr 26, 2020

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Apr 26, 2020

Addresses #2426

This is a refactoring to prepare for a future SEIR model.

  1. Moves the num_quant_bins from .__init__() (which henceforth accept only model-related args) to .fit() (which accepts inference-related args). This reduces the burden of creating a new model class.
  2. Renames SIRModel to SimpleSIRModel and adds documentation that "this model is intended to be forked, not customized"
  3. Fixes definition of timescale tau so that it is actually the mean (there was an off-by-1 error).

Tested

  • added new test configs to test/contrib/epidemiology

computational cost is exponential in `num_quant_bins`. Defaults to 4.
:param int num_quant_bins: The number of quantization bins to use.
Note that computational cost is exponential in `num_quant_bins`.
Defaults to 4.
"""

def __init__(self, population, recovery_time, data, *,
num_quant_bins=4):
Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't see where num_quant_bins is piped here?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, thanks for catching, I've removed.

@martinjankowiak martinjankowiak merged commit a56ecba into dev Apr 26, 2020
@fritzo fritzo deleted the simplify-sir branch April 28, 2020 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants