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

Eliminate need for CompartmentalModel class variables #2521

Merged
merged 3 commits into from
Jun 8, 2020

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Jun 8, 2020

Addresses #2426

This eliminates the need for classes deriving from CompartmentalModel to define class variables .full_mass and .series, thus easing the task of creating new models. After this PR, those class variables are defined automatically via @lazy_property.

This also fixes an issue with sphinx 3.1 that breaks docs building of @lazy_property (this fix is blocking all Pyro development).

Tested

  • refactoring is exercised by existing tests
  • added assertions checking ._full_mass computation
  • ran examples/.../sir.py --plot to verify all series are created and plotted

martinjankowiak
martinjankowiak previously approved these changes Jun 8, 2020
Comment on lines +58 to +61
# This adds a __call__ method to satisfy sphinx.
@patch_dependency('torch.distributions.utils.lazy_property.__call__')
def _lazy_property__call__(self):
raise NotImplementedError
Copy link
Member Author

Choose a reason for hiding this comment

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

This is required for any Pyro development after the recent sphinx 3.1 release.

@martinjankowiak martinjankowiak merged commit 7f9df12 into dev Jun 8, 2020
@fritzo fritzo deleted the sir-rm-cvars branch July 14, 2020 01:24
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