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

Update to more accurate way of calculating ancestors #3213

Merged
merged 1 commit into from
Oct 11, 2018

Conversation

ColCarroll
Copy link
Member

Fixes #3165. There was a theano function I should have been using the whole time (theano.gof.graph.ancestors instead of theano.gof.graph.inputs).

I suspect accurate listing of parents may help with doing sample_prior_predictive (#3210) as well.

Both tricky cases mentioned in the issue are fixed, and here's another extra tricky one that has a nice graph. I am also rechecking the radon notebook now.

with pm.Model() as model:
    a = pm.Normal('a')
    w = pm.Normal('w', a)
    x = pm.Deterministic('x', w)
    y = pm.Deterministic('y', x + a)
    u = pm.Deterministic('u', y + w)
    z = pm.Normal('z', u + y, x)
pm.model_to_graphviz(model)

image

@ColCarroll
Copy link
Member Author

Radon notebook graphs all look identical.

@twiecki
Copy link
Member

twiecki commented Sep 26, 2018

Not sure why travis is complaining, doesn't seem like any tests weren't passing.

There are, however, many warnings (unrelated to this), like:

/home/travis/build/pymc-devs/pymc3/pymc3/distributions/distribution.py:424: DeprecationWarning: The truth value of an empty array is ambiguous. Returning False, but in future this will result in an error. Use `array.size > 0` to check that an array is not empty.
  shape = tuple(shape or ())

@ColCarroll
Copy link
Member Author

Yeah - I think the new numpy is emitting a lot more warnings. I can tackle some of those in a new PR. This might be related that we already fixed: arviz-devs/arviz#271

The job exceeded the maximum log length, and has been terminated.

@ColCarroll
Copy link
Member Author

Is this ok to merge? #3214 may supersede it, but this still fixes some issues.

@twiecki twiecki merged commit 4e33b32 into pymc-devs:master Oct 11, 2018
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 this pull request may close these issues.

model_to_graphviz displays incomplete graph
2 participants