-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Manually set theano TensorType for length 1 shared variables #3335
Manually set theano TensorType for length 1 shared variables #3335
Conversation
- this patch fixes an issue highlighted in pymc-devs#3122 where imputation of a single missing observation fails. It implements the suggestion from the theano error message to manual force a TensorType change in cases where the variable has a length of one.
Thanks @mattpitkin, can you add a test for the bug you ran into? |
@twiecki Where would be best to add the test? For the latter, a test would be, which should fail if my patch has not been applied: import pymc3 as pm
import numpy as np
from scipy.stats import bernoulli
true_p = 0.5
nobs = 10
# draw some observations
obs = bernoulli.rvs(true_p, size=nobs)
# mask one of the observations
obs[0] = -1 # mask first observation
obs = np.ma.masked_values(obs, value=-1)
with pm.Model() as model:
prob = pm.Uniform('prob', 0., 1.0)
pdraws = pm.Bernoulli('draws', p=prob, observed=obs)
trace = pm.sample() |
Yes, in test_model.py with a description and link to this issue. |
- test the error described in pymc-devs#3122 and fixed in pymc-devs#3335.
I've added a test in e93efce. Let me know if that test looks ok. |
Looks great, can you add a note to the release notes? |
Sure, I've added a comment in 2ae431e |
fix typo Co-Authored-By: mattpitkin <[email protected]>
Thanks! |
In #3122 it shows an error caused when trying to impute the values of a single missing variable. The error thrown by theano is, e.g.:
This PR fixes the problem by doing as the theano error suggests. So when shared variables are being set they are checked to see if they are arrays with length 1, and if so the TensorType of the shared variable is changes.
This is a slight hack, but hopefully one that works. I have added a comment above the change in the code, but maybe this should be changed to a
UserWarning
message?