-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
add weighted ppc #2273
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 weighted ppc #2273
Conversation
pymc3/sampling.py
Outdated
@@ -536,6 +544,100 @@ def sample_ppc(trace, samples=None, model=None, vars=None, size=None, | |||
return {k: np.asarray(v) for k, v in ppc.items()} | |||
|
|||
|
|||
def sample_ppc_w(traces, samples=None, models=None, size=None, weights=None, |
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.
IMO Your approach before was better - why adding a new function instead?
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 it could be potentially confusing for the user.
I think is better for the user to have a separated function to perform a weighted ppc (model averaging). |
I see, a notebook demonstrating how to use this for model averaging is necessary then. It would be quite a cool feature. |
I will try do add that notebook ASAP, thanks for the review! |
pymc3/sampling.py
Outdated
raise ValueError( | ||
'The number of traces and weights should be the same') | ||
indices = randint(0, len(trace), samples) | ||
if progressbar: |
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.
tqdm progress bars need to be closed if sampling is interrupted. Some try...finally would do it.
I add an example showing how to use the new function and fix the tqdm issue |
Nice! I think it's ready to merge once the test pass. |
This looks nice! But I think we should merge only after the 3.1 release. |
Fine with me. |
@junpenglao After we fix the notebooks and examples we should be good to go. |
fix error docstring
I just fix a couple of conflicts, that I think were introduced by #2291 |
db0da22
to
e282df2
Compare
I just fix a couple of conflicts (probably in the less elegant way). |
Thanks @aloctavodia |
Allows to use
sample_ppc
to get weighted samples, if more than one trace and a set of weights are passed.The weights could come from the
compare
function, for example.I will try to add a notebook with examples ASAP.