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

Added base class for variational methods #1600

Closed
wants to merge 87 commits into from
Closed

Added base class for variational methods #1600

wants to merge 87 commits into from

Conversation

ferrine
Copy link
Member

@ferrine ferrine commented Dec 13, 2016

Because of upcomming Normalizing flows and Stein variational gradient descent I propose a unified interface for variational methods

fonnesbeck and others added 20 commits November 28, 2016 05:36
…tting (e.g. nanguardmode) for Theano functions
* Started to write Base class for pymc3.models

* mode `add_var` to public api

* Added some docstrings

* Added some docstrings

* added getitem and fixed a typo

* added assertion check

* added resolve var method

* decided not to add resolve method

* Added linear component

* Docs fix

* patsy's intercept is inited properly now

* refactored code

* updated docs

* added possibility to init coefficients with random variables

* added glm

* refactored api, fixed formula init

* refactored linear model, extended acceptable types

* moved useful matrix and labels creation to utils file

* code style

* removed redundant evaluation of shape

* refactored resolver for constructing matrix and labels

* changed error message

* changed signature of init

* simplified utils any_to_tensor_and_labels code

* tests for `any_to_tensor_and_labels`

* added docstring for `any_to_tensor_and_labels` util

* forgot to document return type in `any_to_tensor_and_labels`

* refactored code for dict

* dict tests fix(do not check labels there)

* added access to random vars of model

* added a shortcut for all variables so there is a unified way to get them

* added default priors for linear model

* update docs for linear

* refactored UserModel api, made it more similar to pm.Model class

* Lots of refactoring, tests for base class, more plain api design

* deleted unused module variable

* fixed some typos in docstring

* Refactored pm.Model class, now it is ready for inheritance

* Added documentation for Model class

* Small typo in docstring

* nested contains for treedict (needed for add_random_variable)

* More accurate duplicate implementation of treedict/treelist

* refactored treedict/treelist

* changed `__imul__` of treelist

* added `root` property and `isroot` indicator for base model

* protect `parent` and `model` attributes from violation

* travis' python2 did not fail on bad syntax(maybe it's too new), fixed

* decided not to use functools wrapper

 Unfortunately functools wrapper fails
 when decorating built-in methods so I
 need to fix that improper behaviour.
 Some bad but needed tricks were implemented

* Added models package to setup script

* Refactor utils

* Fix some typos in pm.model
…tting (e.g. nanguardmode) for Theano functions
* Started to write Base class for pymc3.models

* mode `add_var` to public api

* Added some docstrings

* Added some docstrings

* added getitem and fixed a typo

* added assertion check

* added resolve var method

* decided not to add resolve method

* Added linear component

* Docs fix

* patsy's intercept is inited properly now

* refactored code

* updated docs

* added possibility to init coefficients with random variables

* added glm

* refactored api, fixed formula init

* refactored linear model, extended acceptable types

* moved useful matrix and labels creation to utils file

* code style

* removed redundant evaluation of shape

* refactored resolver for constructing matrix and labels

* changed error message

* changed signature of init

* simplified utils any_to_tensor_and_labels code

* tests for `any_to_tensor_and_labels`

* added docstring for `any_to_tensor_and_labels` util

* forgot to document return type in `any_to_tensor_and_labels`

* refactored code for dict

* dict tests fix(do not check labels there)

* added access to random vars of model

* added a shortcut for all variables so there is a unified way to get them

* added default priors for linear model

* update docs for linear

* refactored UserModel api, made it more similar to pm.Model class

* Lots of refactoring, tests for base class, more plain api design

* deleted unused module variable

* fixed some typos in docstring

* Refactored pm.Model class, now it is ready for inheritance

* Added documentation for Model class

* Small typo in docstring

* nested contains for treedict (needed for add_random_variable)

* More accurate duplicate implementation of treedict/treelist

* refactored treedict/treelist

* changed `__imul__` of treelist

* added `root` property and `isroot` indicator for base model

* protect `parent` and `model` attributes from violation

* travis' python2 did not fail on bad syntax(maybe it's too new), fixed

* decided not to use functools wrapper

 Unfortunately functools wrapper fails
 when decorating built-in methods so I
 need to fix that improper behaviour.
 Some bad but needed tricks were implemented

* Added models package to setup script

* Refactor utils

* Fix some typos in pm.model
* Started to write Base class for pymc3.models

* mode `add_var` to public api

* Added some docstrings

* Added some docstrings

* added getitem and fixed a typo

* added assertion check

* added resolve var method

* decided not to add resolve method

* Added linear component

* Docs fix

* patsy's intercept is inited properly now

* refactored code

* updated docs

* added possibility to init coefficients with random variables

* added glm

* refactored api, fixed formula init

* refactored linear model, extended acceptable types

* moved useful matrix and labels creation to utils file

* code style

* removed redundant evaluation of shape

* refactored resolver for constructing matrix and labels

* changed error message

* changed signature of init

* simplified utils any_to_tensor_and_labels code

* tests for `any_to_tensor_and_labels`

* added docstring for `any_to_tensor_and_labels` util

* forgot to document return type in `any_to_tensor_and_labels`

* refactored code for dict

* dict tests fix(do not check labels there)

* added access to random vars of model

* added a shortcut for all variables so there is a unified way to get them

* added default priors for linear model

* update docs for linear

* refactored UserModel api, made it more similar to pm.Model class

* Lots of refactoring, tests for base class, more plain api design

* deleted unused module variable

* fixed some typos in docstring

* Refactored pm.Model class, now it is ready for inheritance

* Added documentation for Model class

* Small typo in docstring

* nested contains for treedict (needed for add_random_variable)

* More accurate duplicate implementation of treedict/treelist

* refactored treedict/treelist

* changed `__imul__` of treelist

* added `root` property and `isroot` indicator for base model

* protect `parent` and `model` attributes from violation

* travis' python2 did not fail on bad syntax(maybe it's too new), fixed

* decided not to use functools wrapper

 Unfortunately functools wrapper fails
 when decorating built-in methods so I
 need to fix that improper behaviour.
 Some bad but needed tricks were implemented

* Added models package to setup script

* Refactor utils

* Fix some typos in pm.model
@twiecki
Copy link
Member

twiecki commented Dec 13, 2016

Seems like you need to rebase.

@ferrine
Copy link
Member Author

ferrine commented Dec 15, 2016

After some tests it will be ready for merge, so I need a review

* refactored GARCH and added Mv(Gaussian/StudentT)RandomWalk

* refactored garch logp

* added docs

* fix typo

* even more typos

* better description for tau

def flatten(tensors, outdim=1):
if not isinstance(tensors, list):
return _flatten(tensors, outdim)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm using a private method that's the same name as the other one. I'm not so sure about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually you've done it quite a lot - so this is just a style thing - I don't see a huge problem with it.

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 was thinking about it too. It should be better named as list_flatten or something like this

samples = tt.as_tensor(samples)
pi = tt.as_tensor(pi)
_elbo_ = self.log_p_D + pi * (self.log_p_W - self.log_q_W)
elbos, updates = theano.scan(fn=lambda: _elbo_,
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this

Copy link
Contributor

Choose a reason for hiding this comment

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

Good use of the theano.scan functionality, and hidden well in the API.

@springcoil
Copy link
Contributor

Code looks clean - a few stylistic concerns but nothing major. @ferrine do you want another review before this merges?

@ferrine
Copy link
Member Author

ferrine commented Dec 15, 2016

Thanks, @springcoil. Yes, I want some more reviews as I want Normalizing Flows and SVGD to be implemented through this unified api. CC @twiecki @taku-y

@springcoil
Copy link
Contributor

Cool. I'd say split this into two PR's it just makes review so much easier :)

@ferrine
Copy link
Member Author

ferrine commented Jan 17, 2017

That was pretty bad merge... So, I've tested scaling and we can ignore scaling densities in the future. This PR probably needs hard refactoring again.

@ferrine
Copy link
Member Author

ferrine commented Jan 17, 2017

Hmm, I think we are able to add recompution of models logp so that it will be evaluated on a number of samples. This way we can avoid costly scan op when sampling logp. It will extend Model's interface btw. I'll add it to PR

@ferrine
Copy link
Member Author

ferrine commented Jan 17, 2017

I'm planning to refactor most of the code as I'm not satisfied with approach I use now. I'll do advi in OPVI framework soon. Implementing LG opvi will be in a couple of lines as well as advi. Here is a draft for supposed opvi interface

@taku-y
Copy link
Contributor

taku-y commented Jan 18, 2017

Sounds great. Hope your refactoring goes well. Btw, I'm curious about removing scan. Is it related to the shape of RV (#1125)?

@ferrine
Copy link
Member Author

ferrine commented Jan 18, 2017

@taku-y, no, it's not related as I want to do elemwise computation on custom tensor. I'm afraid how it will look like on Multi Observed RV

@taku-y
Copy link
Contributor

taku-y commented Jan 18, 2017

@ferrine Thanks for your reply. I shuold wait your code. I look forward to your new framework!

@ferrine
Copy link
Member Author

ferrine commented Jan 19, 2017

I've tried to rebase the branch on master but it was full of pain. I would rather create a new branch and OPVI PR. I have lot's of things to refactor so there will be a mess soon otherwise

@ferrine
Copy link
Member Author

ferrine commented Jan 22, 2017

Closing in favour of #1694

@ferrine ferrine closed this Jan 22, 2017
@ferrine ferrine mentioned this pull request Jan 22, 2017
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.

6 participants