Skip to content

Adding notebook with example of using a "black box" likelihood #3169

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

Merged
merged 3 commits into from
Aug 30, 2018

Conversation

mattpitkin
Copy link
Contributor

This PR adds an example notebook that describes creating a Theano Op for the "black box" likelihood function, including adding a grad method. This has been discussed on the forum here.

The description probably still needs some tidying though!

cc @junpenglao

@junpenglao
Copy link
Member

Requested review from @aseyboldt and @ferrine who have more expert theano knowledge ;-)

Copy link
Member

@ferrine ferrine left a comment

Choose a reason for hiding this comment

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

Looks very good to me. There is nice motivation, somewhat simple solution (deserves a PR), and a kind warning that this might be a last resort method.

What about the performance of logp_grad function? Could you please additionally profile them in the notebook?

@mattpitkin
Copy link
Contributor Author

@ferrine At the end of the notebook I've added some tests that check that the gradient function I define is the same as that for the PyMC3 Normal distribution.

Would you like some profiling of the timings of the gradient function?

@junpenglao
Copy link
Member

I think a sentence or two pointing to the possibility of doing profiling using http://docs.pymc.io/notebooks/profiling.html is enough.
Also, small formatting nitpick: please follow #2834

@mattpitkin
Copy link
Contributor Author

@junpenglao I've added profiling of the logpt function, which in my case also seems to profile the gradient, and included a link to the profiling notebook. I've also made some style changes and tried to adopt some of the formatting suggested in #2834. Should I remove the version info for everything other than PyMC3?

@junpenglao
Copy link
Member

Looks great like this! Thanks a lot!

@junpenglao junpenglao merged commit ed433eb into pymc-devs:master Aug 30, 2018
@mattpitkin mattpitkin changed the title WIP: Adding notebook with example of using a "black box" likelihood Adding notebook with example of using a "black box" likelihood Aug 30, 2018
@mattpitkin mattpitkin deleted the blackbox_likelihood_example branch August 30, 2018 10:37
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.

3 participants