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

Merge variational inference #259

Closed
dritchie opened this issue Oct 23, 2015 · 5 comments
Closed

Merge variational inference #259

dritchie opened this issue Oct 23, 2015 · 5 comments
Milestone

Comments

@dritchie
Copy link
Contributor

Over in my fork of webppl, I have an inference routine that implements various variational inference algorithms (ELBO maximization, EUBO minimization using posterior samples, and variational particle filtering a.k.a. NASMC):

https://github.com/dritchie/webppl/blob/vpf/src/inference/variational.js

According to recent conversation with Noah, one of the next high-priority items for webppl is to get HMC implemented and merged (#81), so I figured it'd be a good time to bring this up, since both HMC and variational inference require gradients. Also, this code could be pretty useful for implementing 'Bayesian adaptive caching.'

AFAIK, there are just a couple of roadblocks to overcome/decisions to be made before this is possible.

  • The ad library I'm using is structured a little bit differently than Sid's, and it also introduces support for tensor-valued variables. We've talked about how to resolve these differences, and it seems totally doable.
  • My code sweet-js transforms files via make, whereas Sid's uses an augmentation to require. I'm totally happy to convert to established practice here.

Any other issues anybody can see?

@null-a
Copy link
Member

null-a commented Oct 29, 2015

My code sweet-js transforms files via make, whereas Sid's uses an augmentation to require.

Getting this working in the browser seems more straight-forward using a build step or similar (perhaps using grunt #248) rather than require-transform. I have a branch where I'm experimenting with merging HMC and while it's far from complete, it is running in the browser. It differs slightly from your approach:

  • Rather than splitting out the "scorers" into a separate file I experimented with a transform which selectively expands AD macros in only the relevant functions.
  • The transformed version of the ERP code isn't checked into Git. Instead the transform is performed by an NPM post install script. With this approach we'd have to manually run the script if erp.ad.js is changed during development, or perhaps automate it with grunt watch.

I don't claim that either of these are better than your solutions, I just wanted to point them out.

We've talked about how to resolve these differences

How can we make this happen? Two suggestions I have are:

  1. I could attempt to integrate AD into WebPPL in a way which is usable by both variational and HMC. (Chatting to @dritchie and @iffsid to figure out the details e.g. tensor support.) We might not get this exactly right, but hopefully once it's done work could proceed largely independently on merging the variational and HMC branches.
  2. We proceed sequentially. I could work on merging HMC such that the AD implementation will be useable for variational inference. Once that's done, the variational branch could be rebased against dev.

@iffsid
Copy link
Contributor

iffsid commented Oct 29, 2015

I chose require-transform simply because it was the easiest path to getting ad running without issue in node. I really like the selective transform idea; keeps the codebase cleaner and does what it should.

I'll push my changes to ad.js soon. I incorporated both @dritchie's and my modifications.
It has support for tensors through numeric.js and has a more elaborate 'setup' object that indicates fwd vs rev mode, 1st vs higher-order derivative, and scalar vs tensor specializations.
I also normalized the implementation to be class-based as in @dritchie's case for efficiency purposes.

For the present, I'd vote on proceeding sequentially ala 2. Should make jointly merging HMC and variational easier (by HMC, I also include SMC with HMC for rejuvenation).

@stuhlmueller
Copy link
Member

The transformed version of the ERP code isn't checked into Git. Instead the transform is performed by an NPM post install script. With this approach we'd have to manually run the script if erp.ad.js is changed during development, or perhaps automate it with grunt watch.

I agree that npm postinstall + grunt watch (or perhaps gulp) would be a good solution.

We proceed sequentially. I could work on merging HMC such that the AD implementation will be useable for variational inference. Once that's done, the variational branch could be rebased against dev.

👍

@dritchie
Copy link
Contributor Author

This all sounds good to me :)

This was referenced Nov 3, 2015
Merged
@null-a null-a modified the milestone: v0.9 Feb 12, 2016
@null-a null-a mentioned this issue Mar 18, 2016
@ngoodman
Copy link
Contributor

ngoodman commented May 7, 2016

I believe variational is fully merged (or re-implemented) in the daipp branch, so i'm closing this for now. (However #27 is still open for merging variational into dev.)

@ngoodman ngoodman closed this as completed May 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants