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.

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

Prior predictive keyword arg not used in from_mcmcchains #146

Closed
bgroenks96 opened this issue Dec 20, 2021 · 5 comments · Fixed by #152
Closed

Prior predictive keyword arg not used in from_mcmcchains #146

bgroenks96 opened this issue Dec 20, 2021 · 5 comments · Fixed by #152

Comments

@bgroenks96
Copy link

When using from_mcmcchains(chain; prior_predictive=prior_pred_chain), the prior predictive chain is not stored in the resulting InferenceData.

Looking at mcmcchains.jl, it's pretty clear why; the keyword arg is listed in the documentation but I don't see it handled anywhere in the code?

@sethaxen
Copy link
Member

Thanks for the issue! from_mcmcchains does handle the prior_predictive group, but it only does so if prior is also provided, which is not right:

ArviZ.jl/src/mcmcchains.jl

Lines 254 to 271 in 17f2e7e

if prior !== nothing
pre_prior_idata = convert_to_inference_data(
prior;
posterior_predictive=prior_predictive,
library=library,
eltypes=eltypes,
kwargs...,
)
prior_idata = rekey(
pre_prior_idata,
Dict(
:posterior => :prior,
:posterior_predictive => :prior_predictive,
:sample_stats => :sample_stats_prior,
),
)
concat!(all_idata, prior_idata)
end

I'll push a fix, though it might wait until after the holidays. For now, you can make it work by providing also the prior chains, which I assume you also have.

@sethaxen
Copy link
Member

@bgroenks96 the latest ArviZ release should resolve this issue. Please let me know if it persists.

@bgroenks96
Copy link
Author

Ok will do. I temporarily stopped using the package, though, because I ran into difficulties with the interface to InferenceData. We really need a better way to handle xarray from Julia.

@sethaxen
Copy link
Member

We really need a better way to handle xarray from Julia.

I agree. Unfortunately, wrapping xarray really well essentially involves writing an XArray.jl package that wraps all xarray types, which is beyond the scope of this package, especially since we're working to drop Python as a dependency (see e.g. #128, #130). Unfortunately, there's really no clear best xarray drop-in alternative in the Julia ecosystem yet.

If you have the time, issues documenting the specific problems would be very useful, because I might be able to add simple workarounds that would make this package more useful in the interim.

@bgroenks96
Copy link
Author

bgroenks96 commented Jan 19, 2022

I would also be happy with a pure-Julia InferenceData scheme. But I doubt you have time to re-implement the multitude of functionality in xarray, though! I certainly don't...

I actually wanted to take a crack at creating an XArray.jl packge. I already have some code lying around which half-implements a wrapping scheme. I hope that I will soon have some time to work on this... I am really not happy with and am constantly bothered by the limitations and instability of DimensionalData.jl, the closest Julia alternative to xarray, at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants