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

Support HaarReparam for non-compartmental variables #2523

Merged
merged 8 commits into from
Jun 13, 2020

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Jun 13, 2020

Addresses #2426

This follows up #2517 to support HaarReparam and SplitReparam for non-compartmental latent time series. After this PR haar_full_mass will trigger learning of covariance among the low frequency components of all time series, hopefully improving mixing in long-duration datasets.

Summary:

  1. Factors out haar and haar_full_mass logic into a _HaarSplitReparam class.
  2. Factors out sampling of all auxiliary variables into a ._sample_auxiliary() method.
  3. Changes non-compartmental variables to avoid plates, thereby permitting reparameterization along the time dimension (we can't reparameterize along a plate dimension; this was the crux).
  4. Adds a new HeterogeneousRegionalSIRModel (ported from Add epidemiology tutorial with a regional SEIR model #2518) to exercise new code.
  5. Cleans up model imports and docs, since this was getting unweildy.
  6. Renames Re to Rt which is more standard.
  7. Bumps pytest version to fix recent pytest_cov breakage.

From @martinjankowiak's comment in #2517 (review):

it really is getting quite complicated though so it's probably good that the current phase of feature addition is coming to an end.

I hope the refactoring in this PR helps to reduce complexity.

Tested

@@ -868,3 +849,77 @@ def init(self, state):
def step(self, state):
with poutine.block(hide_types=["observe"]):
super().step(state.copy())


class _HaarSplitReparam:
Copy link
Member Author

@fritzo fritzo Jun 13, 2020

Choose a reason for hiding this comment

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

@fehiepsi I found this class provides useful helpers to convert between user <--> aux coordinates, in addition to our usual .reparam() functionality. These extra helpers seem necessary for poutine.reparam() to play well with init_to_value(). That is, if we reparameterize a model, then a user's custom init_to_value() function will need to be converted to different coordinates. WDYT of generalizing these to be part of the Reparam interface, say as a pair of methods (.aux_to_user(), .user_to_aux()) or similar? (in follow-up PRs to Pyro and NumPyro).

Copy link
Member

Choose a reason for hiding this comment

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

These extra helpers seem necessary for poutine.reparam() to play well with init_to_value().

I see. I agree that those helper methods would simplify much of the user code for this init strategy.

Comment on lines -12 to -21
"HeterogeneousSIRModel",
"OverdispersedSEIRModel",
"OverdispersedSIRModel",
"RegionalSIRModel",
"SimpleSEIRModel",
"SimpleSIRModel",
"SparseSIRModel",
"SuperspreadingSEIRModel",
"SuperspreadingSIRModel",
"UnknownStartSIRModel",
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing these because they are inteded more as examples than as reusable components.

dist.Uniform(-0.5, self.population + 0.5)
.mask(False).expand(shape).to_event())
assert auxiliary.shape == shape, "particle plates are not supported"
# Split tenors into current state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Copy link
Member Author

Choose a reason for hiding this comment

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

done

# Transform to Haar coordinates.
config = {}
for name, dim in self.dims.items():
config[name] = HaarReparam(dim=dim, flip=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

make flip an arg?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's unnecessary since we don't expose any plumbing to set that, and since _HaarSplitReparam is merely a minimal internal helper.

@martinjankowiak martinjankowiak merged commit f7a5677 into dev Jun 13, 2020
@fritzo
Copy link
Member Author

fritzo commented Jun 14, 2020

Thanks for reviewing!

@fritzo fritzo deleted the sir-noncom-haar 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.

3 participants