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

Debug print left in release version #1560

Closed
rpgoldman opened this issue Feb 13, 2021 · 2 comments · Fixed by #1573
Closed

Debug print left in release version #1560

rpgoldman opened this issue Feb 13, 2021 · 2 comments · Fixed by #1573
Labels

Comments

@rpgoldman
Copy link
Contributor

Describe the bug

While invoking from_pymc3 I got unexpected output:

0, dim: AND rows, 1376 =? 1376
0, dim: NAND rows, 1799 =? 1799
0, dim: NOR rows, 1699 =? 1699
0, dim: OR rows, 1370 =? 1370
0, dim: XNOR rows, 1510 =? 1510
0, dim: XOR rows, 1674 =? 1674

The strings after dim: are names of dimensions in the InferenceData object being created.

I find this in arviz/data/base.py:

print(f"{i}, dim: {dim}, {dim_size} =? {len(coords.get(dim, []))}") 

See

print(f"{i}, dim: {dim}, {dim_size} =? {len(coords.get(dim, []))}")

This line should either be deleted or replaced by an equivalent logging function call.

This line in the code is gated by the parameter skip_event_dims which is not explained in the docstring. See

skip_event_dims : bool, default False

I'm actually not even sure what an event dimension is, as opposed to any other kind of dimension, so some more information about this parameter would be very helpful.

@OriolAbril
Copy link
Member

Extra info on event_dim in addition to the explanations in the PR (#1429) that introduced the changes:

I took the naming from tensorflow probability, they define:

  • Event shape describes the shape of a single draw from the distribution; it may be dependent across dimensions. For scalar distributions, the event shape is []. For a 5-dimensional MultivariateNormal, the event shape is [5].
  • Batch shape describes independent, not identically distributed draws, aka a "batch" of distributions.
  • Sample shape describes independent, identically distributed draws of batches from the distribution family.

So for example, if we model our 50 observations as 3d multivariate normals, and took 4 chains, 200 draws from the posterior, then the shape of the posterior predictive would be (4, 200, 50, 3) with (4, 200) being the sample shape, 50 the batch shape and 3 the event shape. log likelihood however will not have the same shape, as the event shape is reduced. Before that PR, things worked as long as no dims were passed to the converter (the defaults are generated on a per group basis) but would fail if dims were passed given that the variables in posterior predictive and in log likelihood have the same name

@OriolAbril
Copy link
Member

This line should either be deleted or replaced by an equivalent logging function call.

This line should be deleted, it is a remnant of some debugging I did when adding the feature.

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

Successfully merging a pull request may close this issue.

2 participants