Skip to content

Commit

Permalink
Manually set theano TensorType for length 1 shared variables (#3335)
Browse files Browse the repository at this point in the history
* model.py: fix issue with length 1 shared variables
 - this patch fixes an issue highlighted in #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.

* model.py: add a comment about the previous change

* model.py: extra check to deal with test errors

* model.py: changes to the length 1 fix

* test_model.py: check shared tensor type conversion in ValueGradFunction

 - test the error described in #3122 and fixed in #3335.

* RELEASE-NOTES.md: added mention of fix to #3122

* Update RELEASE-NOTES.md

fix typo

Co-Authored-By: mattpitkin <m@ttpitk.in>
  • Loading branch information
mattpitkin authored and twiecki committed Jan 15, 2019
1 parent b64f810 commit f245f11
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Made `BrokenPipeError` for parallel sampling more verbose on Windows.
- 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).
- 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).
- Added a fix to allow the imputation of single missing values of observed data, which previously would fail (Fix issue #3122).

### Deprecations

Expand Down
8 changes: 8 additions & 0 deletions pymc3/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ class ValueGradFunction:
"""
def __init__(self, cost, grad_vars, extra_vars=None, dtype=None,
casting='no', **kwargs):
from .distributions import TensorType

if extra_vars is None:
extra_vars = []

Expand Down Expand Up @@ -437,6 +439,12 @@ def __init__(self, cost, grad_vars, extra_vars=None, dtype=None,
self._extra_vars_shared = {}
for var in extra_vars:
shared = theano.shared(var.tag.test_value, var.name + '_shared__')
# test TensorType compatibility
if hasattr(var.tag.test_value, 'shape'):
testtype = TensorType(var.dtype, var.tag.test_value.shape)

if testtype != shared.type:
shared.type = testtype
self._extra_vars_shared[var.name] = shared
givens.append((var, shared))

Expand Down
13 changes: 13 additions & 0 deletions pymc3/tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,16 @@ def test_edge_case(self):
assert logp.size == 1
assert dlogp.size == 4
npt.assert_allclose(dlogp, 0., atol=1e-5)

def test_tensor_type_conversion(self):
# case described in #3122
X = np.random.binomial(1, 0.5, 10)
X[0] = -1 # masked a single value
X = np.ma.masked_values(X, value=-1)
with pm.Model() as m:
x1 = pm.Uniform('x1', 0., 1.)
x2 = pm.Bernoulli('x2', x1, observed=X)

gf = m.logp_dlogp_function()

assert m['x2_missing'].type == gf._extra_vars_shared['x2_missing'].type

0 comments on commit f245f11

Please sign in to comment.