Skip to content

Commit

Permalink
Replace RVs by respective value variables in the graph of untransform…
Browse files Browse the repository at this point in the history
…ed variables

This fixes incorrect behavior, where deterministic projection from transformed (sampling) to untransformed space would be nonsensical when transforms depend on other graph variables
  • Loading branch information
ricardoV94 authored and twiecki committed Feb 3, 2022
1 parent af6b3c6 commit 2f8f110
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
10 changes: 7 additions & 3 deletions pymc/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -900,10 +900,11 @@ def value_vars(self):
@property
def unobserved_value_vars(self):
"""List of all random variables (including untransformed projections),
as well as deterministics used as inputs and outputs of the the model's
as well as deterministics used as inputs and outputs of the model's
log-likelihood graph
"""
vars = []
untransformed_vars = []
for rv in self.free_RVs:
value_var = self.rvs_to_values[rv]
transform = getattr(value_var.tag, "transform", None)
Expand All @@ -912,13 +913,16 @@ def unobserved_value_vars(self):
# each transformed variable
untrans_value_var = transform.backward(value_var, *rv.owner.inputs)
untrans_value_var.name = rv.name
vars.append(untrans_value_var)
untransformed_vars.append(untrans_value_var)
vars.append(value_var)

# Remove rvs from untransformed values graph
untransformed_vars, _ = rvs_to_value_vars(untransformed_vars, apply_transforms=True)

# Remove rvs from deterministics graph
deterministics, _ = rvs_to_value_vars(self.deterministics, apply_transforms=True)

return vars + deterministics
return vars + untransformed_vars + deterministics

@property
def basic_RVs(self):
Expand Down
11 changes: 11 additions & 0 deletions pymc/tests/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import numpy as np
import numpy.testing as npt
import pytest
import scipy.special

from aesara import shared
from arviz import InferenceData
Expand Down Expand Up @@ -326,6 +327,16 @@ def test_deterministic_of_unobserved(self):

np.testing.assert_allclose(idata.posterior["y"], idata.posterior["x"] + 100)

def test_transform_with_rv_depenency(self):
# Test that untransformed variables that depend on upstream variables are properly handled
with pm.Model() as m:
x = pm.HalfNormal("x", observed=1)
transform = pm.transforms.IntervalTransform(lambda *inputs: (inputs[-2], inputs[-1]))
y = pm.Uniform("y", lower=0, upper=x, transform=transform)
trace = pm.sample(tune=10, draws=50, return_inferencedata=False, random_seed=336)

assert np.allclose(scipy.special.expit(trace["y_interval__"]), trace["y"])


def test_sample_find_MAP_does_not_modify_start():
# see https://github.com/pymc-devs/pymc/pull/4458
Expand Down

0 comments on commit 2f8f110

Please sign in to comment.