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

HMC over Enumerate #344

Merged
merged 14 commits into from
Feb 19, 2016
Merged

HMC over Enumerate #344

merged 14 commits into from
Feb 19, 2016

Conversation

null-a
Copy link
Member

@null-a null-a commented Feb 17, 2016

This addresses #293. There are tests for the use cases mentioned in the issue.

All that was needed to make it work was to use ad primitives in a handful of places in the inference code. Conceptually that's all that's happening here, but it looks much more complicated than that primarily because the code that previously expanded ad macros in erp.ad.js has been generalized to transform any .ad.js file.

The reason I'm not using util.logsumexp in distributions.ad.js is that unlike Math.exp, ad.maths.exp only takes 2 arguments. Once this PR is merged we can open a new issue about fixing that.

I've not switched to transforming all of erp.ad.js to avoid making this PR harder to review.

One thing I'm not happy with is the fact that new .ad.js files need to be added to .gitignore and Gruntfile.js by hand. It's not hard to think of ways to fix this (though I'm not sure which way is best) but I think it's best tackled as a separate piece of work.

@stuhlmueller
Copy link
Member

Looks good! We can merge this as soon as the conflicts are resolved.

I tried the following model to see if HMC helps get a better answer when there are many hyperparameters (100 in this case):

var mean = function(erp) {
  return sum(erp.support()) / erp.support().length;
};

var model = function() {
  var xs = repeat(100, function(){
    return uniform(0, 1);
  });
  var x = xs[0];
  var inner = Enumerate(function(){
    var y1 = flip(x);
    var y2 = flip(x);
    return y1 && y2;
  });
  var z = sample(inner);
  condition(z);
  return x;
};

var run_mh = function(){
  console.log('Running MH');
  return MCMC(model, {
    samples: 3000,
    kernel: 'MH'
  });
};

var run_hmc = function(){
  console.log('Running HMC');
  return MCMC(model, {
    samples: 3000,
    kernel: {
      HMC: {
        steps: 10,
        stepSize: 0.1
      }
    }
  });
};

var mh_results = map(mean, repeat(10, run_mh));
var hmc_results = map(mean, repeat(10, run_hmc));

var results = {
  'true mean:': 3/4,
  'hmc means:': hmc_results,
  'mh means:': mh_results
};

results;

The results do look substantially better for HMC:

{ 'true mean:': 0.75,
  'hmc means:':
   [ 0.7390769632050018,
     0.7495659007818611,
     0.7149450296691322,
     0.7544682914556162,
     0.7768129189201416,
     0.7449967978532908,
     0.7509121363021867,
     0.7695947452269282,
     0.7723961324693488,
     0.7546555283007204 ],
  'mh means:':
   [ 0.7426825795824138,
     0.6136299660429358,
     0.7828223544784123,
     0.6856209646317769,
     0.809652664756868,
     0.6903588371933438,
     0.7732736983129548,
     0.7428055850156982,
     0.641748535555477,
     0.6862876061183799 ] }

HMC does take more than 10x as long for the same number of samples, but still looks a little better even when I run both algorithms for the same amount of time. (For this particular example, rejection works much better than either MCMC algorithm.)

Minor comments:

  • In scripts/adify, I would have expected use of some existing module function to walk the directory tree (maybe file.walk). But the current code is pretty concise, too, so feel free to leave it as it is.
  • In enumerate.ad.js, you changed this.score += score to this.score = this.score + score, and this.numCompletedExecutions++ to this.numCompletedExecutions += 1. Does AD correctly handle +=? The former change seems to imply that it might not.
  • Checking for 'use strict' in adify is a nice touch.

@null-a
Copy link
Member Author

null-a commented Feb 19, 2016

Nice example! I wonder how close to optimal the HMC parameters (steps etc.) are here.

I would have expected use of some existing module function to walk the directory tree

I agree. I had a quick look around but didn't see anything I felt was suitable, so I just wrote the 15 lines. The thing you linked looks OK, but I think what we have is fine too so I'll leave it for now.

you changed this.score += score

Thanks, that was a mistake which I've now fixed. ad does handle +=.

It doesn't handle ++ in all cases, and the change to numCompletedExecutions is required. (I suppose this is actually a limitation of sweet.js rather than ad itself.)

stuhlmueller added a commit that referenced this pull request Feb 19, 2016
@stuhlmueller stuhlmueller merged commit fe8a2c2 into probmods:dev Feb 19, 2016
@null-a null-a deleted the hmc-over-enum branch February 22, 2016 09:39
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

Successfully merging this pull request may close these issues.

2 participants