Closed OriolAbril closed 2 years ago
Hi, I'd like to work on this issue. Can you please assign me to it?
Done! Thanks for your interest in ArviZ, let us know if you have any doubts or there is something not clear enough on the contributing guide
Hello ArviZ team, Is it possible I can be assigned to this issue? Thank you!
Hi, yes, as it has been more than 2 weeks without any activity since the last comment I am assigning the issue to you as indicated in our contributing guidelines
Hello, I have some questions regarding this issue:
What is the difference between the values in compare_dict
and dataset_dict
? Except for the fact that one takes InferenceData only and the other one takes both ElpdData and InferenceData.
https://github.com/arviz-devs/arviz/blob/8bd1de30cbea184c1493f3272fdca8ec1e6bcc8e/arviz/plots/elpdplot.py#L34
https://github.com/arviz-devs/arviz/blob/8bd1de30cbea184c1493f3272fdca8ec1e6bcc8e/arviz/stats/stats.py#L70
Also, in the compare
function, is ics
used to create the ELPData?
I am sorry if these question's are too simple its my first time contributing here. Thank you for your time!
There should be no differences between the two args in terms of functionality nor in terms of names the issue is that there currently are.
Also, in the compare function, is ics used to create the ELPData?
No, ics is the return object of compare which is a pandas dataframe, ELPDData is a custom class to get cool printouts and so for the results of loo
and waic
. compare
currently takes only InferenceData, computes loo or waic to get the ELPDData and then uses eldpdata to generate the ics
dataframe which is it's output. We want to change the possible inputs which will mean that the idata->elpddata won't always be necessary (like it happens already in plot_elpd).
ELPD repr should list all keywords it can access... right? I think it currently does not show these
I am a bit of a python package and github n00b so stopping short of submitting a PR, but here's some code I wrote to replace compare()
in stats.py
(minimally tested). This will allow a mix of ELPPData
or InferenceData
objects to be passed to compare()
, ICs will be computed for the InferenceData as needed.
Hi @derekpowell, thanks for your help. If will be easier for us to check the code if you do a PR. We can help with the process if you need it.
we have an open PR for that actually: https://github.com/arviz-devs/arviz/pull/1690. It might be worth pinging and taking over if there is no response.
Is this feature completed? If not could I be assigned to it?
The PR is still open #1690, you can start working on top of that and submit a new PR finishing the tests and addressing the review comments.
model comparison functions like
compare
andplot_elpd
take a dictionary of inferencedata objects but both should also allow either a dict of inferencedata objects or a dict of elpddata objects (like plot_elpd already does). This will allow users to compute loo/waic outside the call to compare and store the result as elpddata, and then analyze the elpd data, use compare, plot_khat, plot_elpd, whatever they want without needing to recompute the loo/waic every single time.