Skip to content

Commit cdb69ec

Browse files
lucianopazColCarroll
authored andcommitted
[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.
1 parent 254c574 commit cdb69ec

File tree

4 files changed

+230
-202
lines changed

4 files changed

+230
-202
lines changed

RELEASE-NOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Added the `broadcast_distribution_samples` function that helps broadcasting arrays of drawn samples, taking into account the requested `size` and the inferred distribution shape. This sometimes is needed by distributions that call several `rvs` separately within their `random` method, such as the `ZeroInflatedPoisson` (Fix issue #3310).
1212
- The `Wald`, `Kumaraswamy`, `LogNormal`, `Pareto`, `Cauchy`, `HalfCauchy`, `Weibull` and `ExGaussian` distributions `random` method used a hidden `_random` function that was written with scalars in mind. This could potentially lead to artificial correlations between random draws. Added shape guards and broadcasting of the distribution samples to prevent this (Similar to issue #3310).
1313
- Added a fix to allow the imputation of single missing values of observed data, which previously would fail (Fix issue #3122).
14+
- Fix for #3346. The `draw_values` function was too permissive with what could be grabbed from inside `point`, which lead to an error when sampling posterior predictives of variables that depended on shared variables that had changed their shape after `pm.sample()` had been called.
1415
- Fix for #3354. `draw_values` now adds the theano graph descendants of `TensorConstant` or `SharedVariables` to the named relationship nodes stack, only if these descendants are `ObservedRV` or `MultiObservedRV` instances.
1516

1617
### Deprecations

pymc3/distributions/distribution.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,8 @@ def draw_values(params, point=None, size=None):
309309
# param was drawn in related contexts
310310
v = drawn[(p, size)]
311311
evaluated[i] = v
312-
elif name is not None and name in point:
312+
# We filter out Deterministics by checking for `model` attribute
313+
elif name is not None and hasattr(p, 'model') and name in point:
313314
# param.name is in point
314315
v = point[name]
315316
evaluated[i] = drawn[(p, size)] = v
@@ -495,7 +496,7 @@ def _draw_value(param, point=None, givens=None, size=None):
495496

496497
dist_tmp.shape = distshape
497498
try:
498-
dist_tmp.random(point=point, size=size)
499+
return dist_tmp.random(point=point, size=size)
499500
except (ValueError, TypeError):
500501
# reset shape to account for shape changes
501502
# with theano.shared inputs

pymc3/tests/test_sampling.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,32 @@ def test_model_not_drawable_prior(self):
302302
samples = pm.sample_posterior_predictive(trace, 50)
303303
assert samples['foo'].shape == (50, 200)
304304

305+
def test_model_shared_variable(self):
306+
x = np.random.randn(100)
307+
y = x > 0
308+
x_shared = theano.shared(x)
309+
y_shared = theano.shared(y)
310+
with pm.Model() as model:
311+
coeff = pm.Normal('x', mu=0, sd=1)
312+
logistic = pm.Deterministic('p', pm.math.sigmoid(coeff * x_shared))
313+
314+
obs = pm.Bernoulli('obs', p=logistic, observed=y_shared)
315+
trace = pm.sample(100)
316+
317+
x_shared.set_value([-1, 0, 1.])
318+
y_shared.set_value([0, 0, 0])
319+
320+
samples = 100
321+
with model:
322+
post_pred = pm.sample_posterior_predictive(trace,
323+
samples=samples,
324+
vars=[logistic, obs])
325+
326+
expected_p = np.array([logistic.eval({coeff: val})
327+
for val in trace['x'][:samples]])
328+
assert post_pred['obs'].shape == (samples, 3)
329+
assert np.allclose(post_pred['p'], expected_p)
330+
305331
def test_deterministic_of_observed(self):
306332
meas_in_1 = pm.theanof.floatX(2 + 4 * np.random.randn(100))
307333
meas_in_2 = pm.theanof.floatX(5 + 4 * np.random.randn(100))

0 commit comments

Comments
 (0)