-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Remove necessity to define likelihood variable inside model, is done … #2339
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
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
8e7ac20
Remove necessity to define likelihood variable inside model, is done …
hvasbath 60a7575
rename to sample_smc
hvasbath 2970d18
rename to sample_SMC, raise error of RV exists with llk_name, restruc…
hvasbath 19bf9bd
fix test_step
hvasbath 8ea0fa5
let MVNormal Proposal return 2d step matrix, SMC uses this, remove un…
hvasbath e9cc38e
sample name change again
hvasbath f97b8f4
forgot the test_step, fixed ...
hvasbath f5f3d24
restart travis
hvasbath cac698c
Merge branch 'master' into smc_model_llk
hvasbath File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not leave the option for user to define Deterministic likelihood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a deterministic likelihood?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is basically to check if by any chance the likelihood name variable is defined already, repeated initialisation of the same model would also add the Deterministic again into the model, which would result in an error
this is basically a workaround to get the likelihood written into the trace @fonnesbeck
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you add the logp as a sampler statistic? That it would be attached to the trace but didn't clutter the normal variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how would I do that? @aseyboldt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each step method can set
generates_statsto True and define dtypes for the individual values instats_dtypes. You can have a look atnuts.pylines 75 to 87. The step method must then return for each step a tuple(q, [stats]), whereqis the sampled point andstatscontains the statistics it wants to store.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that feature didnt exist when I started writing SMC, I guess now it could be even possible to get it into the common pm.sample API ... however, that would require some major changes I have no time to do that in the near future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aseyboldt I didnt know about that as well!