add a draft for traces #139

ferrine · 2019-09-06T09:45:35Z

Adding a draft for traces. Previously, traces were returned in a raw format. This PR should solve this issue and ease plotting

rpgoldman · 2019-09-06T16:09:49Z

One thing I would love to see in PyMC4 is a more clear notion of "what is a trace?" It's problematic -- especially when incorporating a model and inference into a larger data flow -- that PyMC3 has at least two kinds of trace: MultiTrace and the trace dictionaries that are returned by sample_posterior_predictive and sample_prior_predictive. (There probably are more, since there are the MAP estimators and variational inference algorithms, but I have not used these. PyMC3 backends are also a locus of inconsistency.)
It would be very helpful to have only a single sort of thing that is created by performing inference.
If this cannot be managed, then we should specify a protocol that indicates how these entities can be accessed, and what comes out of them when we do access them. For example, slicing should always work; if we keep the dictionaries, we should extend them to have attributes that align with those of the MultiTrace (e.g., they should support the chains interface, but always have only one chain, support the points interface, etc.).

junpenglao · 2019-09-06T16:35:38Z

pymc4/inference/utils.py

+    arviz.data.inference_data.InferenceData
+    """
+    import arviz as az
+    az_dict = {k: np.swapaxes(v.numpy(), 1, 0) for k, v in pm4_trace.items()}


It could potentially be problematic if you have > 1d batch shape: arviz-devs/arviz#456 (comment)

But I guess in this case it is fine since we batch the log_prob ourself and the num_chain is only 1d

That already works: <xarray.DataArray 'hierarchical_model/beta' (chain: 50, draw: 200, hierarchical_model/beta_dim_0: 85)>

twiecki · 2019-09-06T16:59:13Z

@rpgoldman I think we should just output arviz trace objects everywhere and use that standard.

rpgoldman · 2019-09-06T17:02:31Z

@rpgoldman I think we should just output arviz trace objects everywhere and use that standard.

Sounds good to me! A good bit of pain in trying to speed up the posterior predictive sampling was not knowing what sort of arguments could be passed in as the trace (and the fact that the tests used some odd ones, like a list made up of the test_point). That makes the code unnecessarily complex and hard to maintain.

canyon289 · 2019-09-07T16:19:05Z

@rpgoldman I think we should just output arviz trace objects everywhere and use that standard.

ArviZ technically doesn't have a trace object. I would say you want to output az.InferenceData objects which includes trace, but can also include posterior predictive, prior predictive, diagnostics etc

canyon289 · 2019-09-07T16:20:48Z

pymc4/inference/utils.py

+    """
+    Tensorflow to Arviz trace convertor.
+
+    Convert a PyMC4 trace as returned by sample() to an ArviZ trace object


Would change this to to an az.InferenceData object or Arviz InferenceData object

I am not sure, this is a bit too specific. Let me try to see if I can add a helper class in TFP so that the output is a bit more standardized.

Add types (including return type)? This information seems to be available in the docstrings...

canyon289 · 2019-09-07T16:21:06Z

pymc4/inference/utils.py

+
+    Returns
+    -------
+    arviz.data.inference_data.InferenceData


This can be shortened to az.InferenceData for end users

canyon289 · 2019-09-07T16:26:10Z

pymc4/distributions/tensorflow/continuous.py

-"""
+"""PyMC4 continuous random variables for tensorflow."""
 import tensorflow_probability as tfp
 from pymc4.distributions import abstract


being picky here, should we use relative imports for inside the library? Not part of this PR but just wanted to ask

I use doctest to test code snippets in documentation. Doctest is complaining if import is relative sometimes

I had that problem with pytest, too -- complaints about relative imports -- and it turned out for me it was because I was running the tests (this is for PyMC3) inside the source directory. When I ran it "above" the directory (i.e., from pymc3 instead of pymc3/pymc3/) the complaints about relative imports went away.

I think the advantage of relative imports is that they don't risk an import cycle as much as absolute ones do.

I think the advantage of relative imports is that they don't risk an import cycle as much as absolute ones do.

I believe this part is true, that relative imports risk less that another package of same name will be imported from sys.path versus the adjacent module

https://stackoverflow.com/a/4209771

My personal feeling is that it is hard to run into a problem you describe, you would not import pymc4 in the first place. But for our use case we can test code snippets in docs without mess

No worries, for this PR just ignore me there's more important things :) thanks @ferrine

rpgoldman · 2019-09-07T19:31:44Z

pymc4/inference/utils.py

+from .. import Model, flow


 def initialize_state(model: Model, observed: Optional[dict] = None) -> flow.SamplingState:


Maybe refine the type declaration here? I.e., change to Optional[Dict[x, y]] for some x and y? Or declare a type for this kind of dictionary, e.g., ObsDict = Dict[x, y]?

There is nothing special about observed except keys are strings. The API is not that narrow at this point.

I think that would be Dict[str, Any] then.
That way we know the keys are names, rather than variables (and so does mypy).

rpgoldman · 2019-09-07T19:32:30Z

pymc4/inference/utils.py

+    """
+    Tensorflow to Arviz trace convertor.
+
+    Convert a PyMC4 trace as returned by sample() to an ArviZ trace object


Add types (including return type)? This information seems to be available in the docstrings...

canyon289 · 2019-09-09T00:46:42Z

Merging per slack conversation, Thanks @ferrine and @twiecki

ferrine and others added 2 commits September 6, 2019 12:44

add a draft for traces

8361ae1

Remove stats tracing which takes time and is deprecated for all RVs.

6fe8035

Add trace conversion to arviz. Add radon example.

3547133

junpenglao reviewed Sep 6, 2019

View reviewed changes

fix lint issues

3541d1d

junpenglao added 2 commits September 7, 2019 00:41

Add diag mass matrix adaptation, diagnostic, and more plots

fa30215

small modification

4b3ecb6

canyon289 reviewed Sep 7, 2019

View reviewed changes

rpgoldman reviewed Sep 7, 2019

View reviewed changes

Add radon file.

7b144fa

twiecki merged commit ed54d88 into master Sep 9, 2019

junpenglao deleted the traces branch October 6, 2019 08:37

lucianopaz mentioned this pull request Dec 19, 2019

Advanced usage: allow the users to deal with manually batching models #193

Merged

		from .. import Model, flow


		def initialize_state(model: Model, observed: Optional[dict] = None) -> flow.SamplingState:

Uh oh!

add a draft for traces #139

add a draft for traces #139

Uh oh!

Conversation

ferrine commented Sep 6, 2019

Uh oh!

rpgoldman commented Sep 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twiecki commented Sep 6, 2019

Uh oh!

rpgoldman commented Sep 6, 2019

Uh oh!

canyon289 commented Sep 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpgoldman Sep 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canyon289 commented Sep 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

canyon289 commented Sep 7, 2019 •

edited

Loading

rpgoldman Sep 7, 2019 •

edited

Loading