-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a method to compile an aesara forward sampling function #5759
Conversation
|
Codecov Report
@@ Coverage Diff @@
## main #5759 +/- ##
=======================================
Coverage 89.27% 89.27%
=======================================
Files 74 74
Lines 13803 13813 +10
=======================================
+ Hits 12322 12332 +10
Misses 1481 1481
|
67c2a13
to
75a3b4a
Compare
7ce1a0f
to
10e06bc
Compare
@ricardoV94, this is ready for review now. We'll address the |
Looks good, just trying to push to simplify code as much as possible. Left some comments above @lucianopaz |
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.
Tested some simplifications locally, do you spot any issues?
10e06bc
to
c346391
Compare
c346391
to
b27a40c
Compare
b27a40c
to
483dbfb
Compare
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.
Awesome @lucianopaz! This was actually one of the trickiest blockers for V4
Closes #5302
This PR adds a low level method to compile the aesara function responsible for generating forward samples. It introduces the notion of volatile variables in a graph, which allows it to determine which values it should take from an inferred posterior during forward sampling. Volatile variables are variables whose values could change between a run of
pm.sample
and future calls topm.sample_posterior_predictive
. These are:vars_in_trace
list (this means that they don't have values in the inferred posterior, so they should be drawn from their a priori assumed distribution)givens_dict
, because setting values throughgivens
is considered disruptive with respect to the values the variable could have taken duringpm.sample
.Tasks that remain before this PR is ready to merge:
compile_forward_sampling_function
insample_prior_predictive
- [ ] Usecompile_forward_sampling_function
insample_posterior_predictive_w