-
Notifications
You must be signed in to change notification settings - Fork 154
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
NLE with multiple iid conditions #1331
Conversation
- deprecate MNLE-based potential (can be nle-based) - adapt tests for conditioned mnle.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1331 +/- ##
===========================================
- Coverage 89.39% 78.41% -10.98%
===========================================
Files 118 118
Lines 8709 8748 +39
===========================================
- Hits 7785 6860 -925
- Misses 924 1888 +964
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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 am not sure I fully understand tbh.
IIUC, then there are four different dimensions across which we could vectorize:
- Across the x batch-dimension, in case we want to amortize.
- Across the x iid-dimension, in case we have iid samples.
- Across a batch of theta conditions which act as different experimental conditions
- Across thetas, in case we want to run multi-chain.
Which one of these does this PR solve? And which ones does it not yet solve?
Can we maybe somewhere define names for each of these four dimensions above and specify which ones are handled by which function (or which ones are currently assumed to be the same dimension and therefore do not have a cartesian product applied to them)?
As discussed in the call, the plan is to
|
@dgedon this is ready for your review. Do let me know if anything is unclear. Thanks! |
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.
Had a bit of a look into it. The test should be fixed, but otherwise looks great. Only left some minor comments on naming and docstrings.
Scenario: we are using NLE in a use-case where
x_o
consists of i.i.d. trials, and our simulator has both parameters and experimental conditions (both contained intheta
). Thus, we are training an emulator on the full simulator. Then, at inference time, we want to additionally condition on a set of experimental conditions and perform inference only over the parameters. To do so, we define a new potential function that fixes those columns intheta
that correspond to the observed experimental conditions and leaves the rest oftheta
free for inference.Problem: So far, this was implemented only for a single experimental condition, i.e., a batch size of one. In practice, however, we often have a batch i.i.d. trials in
x_o
and a matching batch of i.i.d. conditions (e.g., varying difficulty as experimental conditions in a decision-making experiment). This scenario is not supported in the current implementation because theConditionedPotential
allows only a single value as a condition.Solution: This PR introduces a new method to the$\log ; L(\theta | [ x_0, \ldots, x_N], [c_0, \ldots, c_N])$ that is needed to obtain the posterior over the parameters given observed i.i.d. trials and matching i.i.d. experimental conditions.
LikelihoodBasedPotential
(because this case is specific for NLE), that allows to condition on a batch of conditions that matches the batch of i.i.d.x_o
. It uses the underlying density estimator to evaluate the log likelihoodcondition_on_theta
method for likelihood-estimator-based potential to condition on a batch of theta values (experimental conditions) that match the current batch of iidx_o
.