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

Prediction on New Data Fails with Deterministic Variable #3346

Closed
BrianMiner opened this Issue Jan 18, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@BrianMiner
Copy link

BrianMiner commented Jan 18, 2019

I am not sure if this behavior is due to some theoretical limitation or not.....but the prediction of new data with sample_posterior_predictive() fails to recognize new new shared variables when the model was run with a Deterministic() parameter in the model. See example below from the quick start.

import theano

x = np.random.randn(100)
y = x > 0

x_shared = theano.shared(x)
y_shared = theano.shared(y)

with pm.Model() as model:
    coeff = pm.Normal('x', mu=0, sd=1)
    
    # Uncomment this and comment next line to see the issue
    #logistic = pm.Deterministic('p',pm.math.sigmoid(coeff * x_shared))
    logistic = pm.math.sigmoid(coeff * x_shared)
    
    pm.Bernoulli('obs', p=logistic, observed=y_shared)
    trace = pm.sample()
    
    
x_shared.set_value([-1, 0, 1.])
y_shared.set_value([0, 0, 0]) # dummy values

with model:
    post_pred = pm.sample_posterior_predictive(trace, samples=500)

post_pred['obs'].shape
@lucianopaz

This comment has been minimized.

Copy link
Contributor

lucianopaz commented Jan 21, 2019

You're right. It's a bit disappointing not to be able to store p into the trace and then change the shared's shape. The thing is that if you use logistic = pm.Deterministic(...) the values computed for logistic while sampling the posterior, get stored into the trace. These are stored as numpy.ndarrays and their shape is given by the x_shared.shape's at the time you call pm.sample. If you checkout the stored values shape with trace['p'].shape you'll see that you get (2000, 100), with the last axis being equal to len(x). The shape of the parameter's value stored in the trace then has precedence over the shape of the shared array.

Currently, the only ways around this are:

  1. To not use the pm.Deterministic, just as you did.
  2. To remove p from the trace after sampling by calling trace.remove_values('p'). Note that this may be dangerous so it is better to work on a deepcopy of the original trace.
@dsvolk

This comment has been minimized.

Copy link

dsvolk commented Jan 23, 2019

This issue seems to be introduced in the recent pymc3==3.6 update. Try downgrading to pymc3==3.5.

With pymc3==3.5, I've been experimenting with predictive sampling of a model with a Deterministic for a few months, and everything worked fine.

@dsvolk

This comment has been minimized.

Copy link

dsvolk commented Jan 24, 2019

Tested this example with pymc3==3.5 and it works fine both with Deterministic and without:

import numpy as np
import theano
import pymc3 as pm

print('Starting...')
x = np.random.randn(100)
y = x > 0

x_shared = theano.shared(x)
y_shared = theano.shared(y)

with pm.Model() as model:
    coeff = pm.Normal('x', mu=0, sd=1)
    
    # Uncomment this and comment next line to see the issue
    #logistic = pm.Deterministic('p',pm.math.sigmoid(coeff * x_shared))
    logistic = pm.math.sigmoid(coeff * x_shared)
    
    pm.Bernoulli('obs', p=logistic, observed=y_shared)
    trace = pm.sample()
    
    
x_shared.set_value([-1, 0, 1.])
y_shared.set_value([0, 0, 0]) # dummy values

with model:
    post_pred = pm.sample_ppc(trace, samples=500)

print(post_pred['obs'].shape)

lucianopaz added a commit to lucianopaz/pymc3 that referenced this issue Jan 24, 2019

Fix for pymc-devs#3346. draw_values had a statement that said that if…
… the name of a variable was in the supplied point, then the value for said variable would be taken from the point dictionary. This was too permissive, and different from what was done in _draw_value. There, the value could be taken from the point dictionary only if the variable had the attribute model, i.e. it was an RV instance instead of a Deterministic.
@lucianopaz

This comment has been minimized.

Copy link
Contributor

lucianopaz commented Jan 24, 2019

Thanks @dsvolk! I looked at the differences between the released draw_values function and found that I had included a statement that was too permissive with what could be taken from the supplied point. I will shortly submit a PR with the change.

ColCarroll added a commit that referenced this issue Jan 25, 2019

[MAINT] Fix for #3346 (#3352)
* Fix for #3346. draw_values had a statement that said that if the name of a variable was in the supplied point, then the value for said variable would be taken from the point dictionary. This was too permissive, and different from what was done in _draw_value. There, the value could be taken from the point dictionary only if the variable had the attribute model, i.e. it was an RV instance instead of a Deterministic.

* Changed expected SMC exact step values.

* Added comment explaining why we check for the model attribute of parameters to get their values from the point dictionary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment