-
Notifications
You must be signed in to change notification settings - Fork 16
Integration with ArviZ #35
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
Conversation
8d48e5a to
9b0c1e7
Compare
|
I'll try to keep an eye and help wherever possible, I should have time to review and answer questions about InferenceData. |
|
Thanks! I'm doing a lot of code reading and trial/error at the moment, I'll ping you when I have something tangible! |
d319196 to
a320664
Compare
|
@OriolAbril Several of the metrics in It can be problematic when users are first iterating in a model and only need the So I was wondering if you thought overriding some of the class' attributes with a I'll first finish this draft precomputing everything and would implement it when I think I have a good design. |
|
Not sure I understand, you mean so that I think we have not considered it (could be wrong though). What we currently have for the |
|
Not sure I understand, you mean so that `lp` and other sample stats are
available but only computed whenever needed?
Yes, I could do that by defining a `sample_stats` method in my `Trace`
class that overrides the corresponding attribute in `InferenceData`.
Do I make sense?
I think we have not considered it (could be wrong though).
I don't necessarily think you would need to do that on your end.
What we currently have for the `log_likelihood` (it is useful for
loo/waic but otherwise requires extra memory and extra computing) group
is an argument to `from_pymc3`, `from_pyro`... so that users can choose
whether or not the group is to be included in the resulting
InferenceData. The same could be done for sample_stats and its
variables.
I did not see that, I will check it out!
|
|
Nevermind, I ran some simple benchmarks and it looks like the running time is only marginally affected by all the conversions. I believe I also found a way to get the Edit: It is free in terms of computation, but it does add complexity in the inference core that I am not willing to add if I don't have to. However, it is possible and might make sense to get it when I compute the value of deterministic variables. I guess that's one issue with having too much freedom 🤷♂️ |
|
I can confirm that all plots / stats that take an |
We add the warmup samples, warmup sampling info and warmup informations (such as inverse mass matrix and step size) to the trace.
|
Just implemented concatenation. I will now add |
|
@OriolAbril if you want to have a look, the magic happens in trace.py. Everything that should work with ArviZ does, so does the (inplace) addition. I just need to add the |
e56c92c to
a5b5b76
Compare
8973cb4 to
6eee6fc
Compare
|
Found a performance issue linked to the fact that |
ArviZ is the best package for the exploratory analysis of models, it makes sense to provide a seamless integration with the library. I chose to implement the
Traceobject as a subclass of Arviz'sInferenceDataso MCX traces can be directly used in ArviZ.There is a loss of information in that translation since the sampler returns pretty much all there is to know about the process, but we can get that information (say for debugging) using iterative sampling.
lax.scanloop;io_numpyro.py;InferenceDatainstance with chain positions. Test plotting.log_likelihoodas a@property(it requires extra computation).HMCInfoappendmethod to add a single sample to the trace. Should be fast.