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.

Sign up for GitHub

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

Add sample_prior function #2876

Closed
wants to merge 9 commits into from
Closed

Add sample_prior function #2876

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e99207d
Add sample_prior function
ColCarroll Feb 26, 2018
6e75ed6
Allow sample_prior with transformed distributions
ColCarroll Feb 28, 2018
99f71a3
Update and tidy release notes
ColCarroll Feb 28, 2018
8c0d9b3
Fix speed problem
ColCarroll Feb 28, 2018
ea3026b
Works with deterministic variables now
ColCarroll Mar 4, 2018
16c0b3a
Update test
ColCarroll Mar 4, 2018
9a80d0d
Update sampling to use `draw_values`
ColCarroll Mar 25, 2018
b45d6e7
Actually add givens
ColCarroll Mar 26, 2018
4541d16
Incorporate comments, make more readable
ColCarroll Mar 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pymc3/distributions/distribution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ def draw_values(params, point=None, size=None):
missing_inputs = set(params)
while to_eval or missing_inputs:
if to_eval == missing_inputs:
raise ValueError('Cannot resolve inputs for {}'.format([str(j) for j in to_eval]))
raise ValueError('Cannot resolve inputs for {}'.format([str(params[j]) for j in to_eval]))
to_eval = set(missing_inputs)
missing_inputs = set()
for param in to_eval:
try: # might evaluate in a bad order,
evaluated[param] = _draw_value(params[param], point=point, givens=givens.values(), size=size)
if any(param in j for j in named_nodes_children.values()):
givens[param.name] = (params[param], evaluated[param])
if any(params[param] in j for j in named_nodes_children.values()):
Copy link
Contributor

@lucianopaz lucianopaz Mar 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Oops I actually commented on an older commit so it shows as outdated, sorry for the mess. First off, it looks very nice, however I think this line is confusing, You're trying to see if the node params[param] is a child of some other named node. If params[param] were to be a named node, that information should be available in the dictionary named_nodes_parents. If params[param] were not to be a named node, then it would not be registered neither in the named_nodes_parents nor the named_nodes_children dictionaries.
If params[param] is a named node, you should be able to replace this line by:
if named_nodes_parents[params[param]]:
If params[param] is not a named node, then I think it shouldn't bee added to givens but I may be overlooking something.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is much nicer, thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

(also just checked out travis, and your suggestion will also fix failing tests)

givens[params[param].name] = (params[param], evaluated[param])
except theano.gof.fg.MissingInputError:
missing_inputs.add(param)

Expand Down