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

Stan #67

Merged
merged 102 commits into from
Apr 1, 2017
Merged

Stan #67

merged 102 commits into from
Apr 1, 2017

Conversation

tyarkoni
Copy link
Contributor

@tyarkoni tyarkoni commented Feb 23, 2017

This PR adds a new StanBackEnd that should (in theory) do almost everything that the PyMC3BackEnd does. Using it is as simple as specifying backend='stan' when initializing a new Model. The StanBackEnd wraps PyStan, but converts the sampling output into a PyMC3 MultiTrace object, which allows us to use all of the existing plotting/summarizing machinery (at least in principle; in practice, I had to hack my way around the normal MultiTrace initialization process, so some attributes are probably not set properly).

There are some things to iron out before we can merge this. @jake-westfall, maybe you can take a pass at these? We need to do the following:

  • Test everything more thoroughly to make sure it works.

  • Modify at least a few of the existing tests to run using the Stan backend. It's probably also worth adding a couple of tests that run with both backends and make sure the resulting estimates are close. We also need to modify the travis config to install PyStan (and maybe add an optional_dependencies.txt file containing PyStan that people can pip install -r from).

  • Add support for all of the PyMC3 distributions we currently support. The trick here is to map PyMC3 distribution names and arguments onto the Stan language. This is accomplished via the dists dictionary in StanBackEnd, which maps the PyMC3 dist names onto dictionaries containing the Stan names, argument order, and (optionally) and bounds to impose. The latter is required because in Stan there are no separate distributions for clipped distributions; e.g., half-cauchy is just cauchy with a lower bound of 0. In principle I think all we need to do is add new entries to the dists dictionary; I don't think any of the PyMC3 distributions we currently support should introduce any new complications.

  • One gotcha right now is that PyStan ignores the incl_warmup argument in extract() if permuted=True. This is reasonable in one sense, because the permuted results concatenate all chains together, and it's arguably weird to include the different initialization points. But it's a problem for us, because to make results commensurable with the PyMC3BackEnd, we should really include the burn-in and let the user drop it themselves later. The solution is unfortunately probably going to require us to duplicate some of the PyStan code for reformatting the Stan output, but with the burn-in included.

  • Add support for different error distributions. Right now the StanBackEnd only supports normally distributed errors, but we should at minimum support the options we support with PyMC3. This shouldn't take much work; I think we just need to modify the specification of the yhat term and the model error.

@tyarkoni
Copy link
Contributor Author

The tests are passing on Python3 but failing on Python2 on travis. The pymc3 import seems to be failing on Python2; not sure if this is a temporary glitch somewhere or something we need to address, but I guess we can wait and see.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.08%) to 87.772% when pulling d8d1563 on stan into 2caf086 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.02%) to 87.832% when pulling d8d1563 on stan into 2caf086 on master.

@tyarkoni
Copy link
Contributor Author

Couldn't figure out the Python2 import issue, but it goes away if I move all the BackEnd classes back into a single module. So I went with that for now.

@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Coverage decreased (-7.9%) to 87.928% when pulling 62c4da9 on stan into 2caf086 on master.

@coveralls
Copy link

coveralls commented Feb 24, 2017

Coverage Status

Coverage decreased (-7.9%) to 87.906% when pulling 6496de6 on stan into 2caf086 on master.

@coveralls
Copy link

coveralls commented Feb 27, 2017

Coverage Status

Coverage decreased (-7.8%) to 87.5% when pulling ca6431e on stan into 5030c85 on master.

jake-westfall and others added 13 commits February 27, 2017 14:47
There was a problem in the admittedly unusual case where the fixed effects
had cell-means coding, but the random effects for that same factor had
k-1 dummy coding (e.g., 0+threecats in the fixed effects, 1+threecats in the
random effects). This was actually a problem before too, but the prior was
just being silently handled in a not-quite-appropriate way. Now it is
handled appropriately.

The remaining failing tests should be mostly related to consistency between
pymc3 and stan.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 94.441% when pulling bb763c0 on stan into 9560095 on master.

@coveralls
Copy link

coveralls commented Mar 26, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.65% when pulling 2cc5e1a on stan into 9560095 on master.

@coveralls
Copy link

coveralls commented Mar 27, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.65% when pulling bd1d513 on stan into 9560095 on master.

@coveralls
Copy link

coveralls commented Mar 27, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.65% when pulling 875b6d9 on stan into 9560095 on master.

@coveralls
Copy link

coveralls commented Mar 29, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.65% when pulling 139b33f on stan into 9560095 on master.

@jake-westfall
Copy link
Collaborator

jake-westfall commented Mar 30, 2017

The updated images in the README are not displaying correctly in the Stan branch, but they'll display correctly after merging into master.

@coveralls
Copy link

coveralls commented Mar 30, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.65% when pulling 8bb3cb1 on stan into 9560095 on master.

@coveralls
Copy link

coveralls commented Apr 1, 2017

Coverage Status

Coverage decreased (-0.7%) to 94.657% when pulling 570091c on stan into 9560095 on master.

@tyarkoni tyarkoni merged commit e1abd5d into master Apr 1, 2017
@tyarkoni
Copy link
Contributor Author

tyarkoni commented Apr 1, 2017

Not sure why tests are failing on travis; they were passing on the stan branch just before the merge. They also pass locally.

@jake-westfall
Copy link
Collaborator

Looks like this is discussed here, and is fixed on the pymc3 master as of 9 days ago. But evidently not on the pip version. Any thoughts about (or is it even possible to) making travis install pymc3 from github instead of pip? I'm really not sure how many of our users would be installing the pymc3 dependency from pip vs. github, especially since we give instructions for both ways in our README.

@tyarkoni
Copy link
Contributor Author

tyarkoni commented Apr 5, 2017

It's certainly possible to install pymc3 from master on travis, but the problem with doing that is we then won't know if the current PyPI release fails, which is what most people will be using, and is what we should probably support. We could put both master and latest PyPI in the travis matrix, which would cover all the bases.

FWIW, it looks like the fix for that issue is in the latest PyPI release (from 3/30), which was before I merged the Stan branch... so I'm not sure what's going on. Also doesn't explain why the tests passed in the stan branch but not when merged. Let's see if the currently running tests pass.

@twiecki
Copy link
Contributor

twiecki commented Apr 5, 2017 via email

@tyarkoni
Copy link
Contributor Author

tyarkoni commented Apr 5, 2017

@twiecki cool, thanks for the update. We'll probably just sit tight and wait for that then, and in the unlikely event that anyone has problems in the meantime, we'll tell them to update from master.

@jake-westfall jake-westfall deleted the stan branch April 10, 2017 20:49
jake-westfall pushed a commit that referenced this pull request Sep 29, 2017
Release 0.1.0 is here! Full details can be found in the [Changelog](https://github.com/bambinos/bambi/blob/master/Changelog.md).
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.

4 participants