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

Add an EasyGuide module, more flexible than AutoGuide #1888

Merged
merged 12 commits into from May 31, 2019

Conversation

Projects
None yet
3 participants
@fritzo
Copy link
Collaborator

commented May 28, 2019

This adds a pyro.contrib.easyguide module that aims to be simpler and more flexible than our original pyro.contrib.autoguide. The main differences are:

  1. EasyGuide supports data subsampling, relying on param subsampling #1796.
  2. EasyGuide is a framework in which users write a guide method using standard Pyro syntax. Whereas AutoGuides are composed using AutoGuideList, EasyGuide achieves composition by creating multiple group.sample() statements in the guide method.
  3. EasyGuide exposes machinery to make it simpler to define amortized guides, specifically machinery to amortize multiple sample sites from a single nn and to batch those nn evaluations over a plate.

One major design decision was to avoid AutoGuide's pattern of hiding pyro.param statements. In practice I've found initialization to be crucial for convergence in SVI. By exposing pyro.param statements, we force the user to create initial param values.

Tested

  • added unit tests
  • added a tutorial

@fritzo fritzo added this to the 0.4 release milestone May 28, 2019

@fritzo fritzo requested a review from martinjankowiak May 28, 2019

Show resolved Hide resolved pyro/contrib/easyguide/easyguide.py
Show resolved Hide resolved pyro/contrib/easyguide/easyguide.py Outdated
self._guide = weakref.ref(guide)
self.sites = sites

# A group is in a frame only if all its sample sites are in that frame.

This comment has been minimized.

Copy link
@martinjankowiak

martinjankowiak May 28, 2019

Collaborator

what happens in 176ff if a group of sites crosses a frame boundary?

This comment has been minimized.

Copy link
@fritzo

fritzo May 28, 2019

Author Collaborator

Then self.frames will be set to only those frames populated by all sites. If a frame is occupied by a strict subset of sites (and hence not in self.frames) then than frame cannot be subsampled (if it is subsampled, an error will be raised).

This comment has been minimized.

Copy link
@martinjankowiak

martinjankowiak May 28, 2019

Collaborator

where exactly does that error get triggered?

This comment has been minimized.

Copy link
@fritzo

fritzo May 30, 2019

Author Collaborator

Hmm good point. I've added an informative error message, as well as two new tests:

  1. test that it's ok to mix shared and non-shared plates if all shared plates are left of non-shared plates.
  2. test that the error is triggered if a shared plate is right of a non-shared plate.

This comment has been minimized.

Copy link
@martinjankowiak

martinjankowiak May 30, 2019

Collaborator

thanks for the additional tests! i'm still a bit confused about this. would like to discuss offline tomorrow before merging

Show resolved Hide resolved tutorial/source/easyguide.ipynb Outdated
@CLAassistant

This comment has been minimized.

Copy link

commented May 29, 2019

CLA assistant check
All committers have signed the CLA.

@martinjankowiak
Copy link
Collaborator

left a comment

thanks this is awesome!

@martinjankowiak martinjankowiak merged commit a5caab6 into dev May 31, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
license/cla Contributor License Agreement is signed.
Details

@martinjankowiak martinjankowiak deleted the easyguide2 branch May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.