From d06eadf5b9ee8a956c14912da73bd0376d87341a Mon Sep 17 00:00:00 2001 From: Ricardo Date: Fri, 15 Jan 2021 09:23:30 +0100 Subject: [PATCH 1/4] Raise warning when sampling with Potentials --- pymc3/sampling.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pymc3/sampling.py b/pymc3/sampling.py index 0dedb87723..cd3db3233f 100644 --- a/pymc3/sampling.py +++ b/pymc3/sampling.py @@ -1692,6 +1692,13 @@ def sample_posterior_predictive( model = modelcontext(model) + if model.potentials: + warnings.warn( + "The effect of Potentials on other parameters is ignored during posterior predictive sampling. " + "This is likely to lead to invalid or biased predictive samples.", + UserWarning, + ) + if var_names is not None: vars_ = [model[x] for x in var_names] else: @@ -1791,6 +1798,15 @@ def sample_posterior_predictive_w( if models is None: models = [modelcontext(models)] * len(traces) + for model in models: + if model.potentials: + warnings.warn( + "The effect of Potentials on other parameters is ignored during posterior predictive sampling. " + "This is likely to lead to invalid or biased predictive samples.", + UserWarning, + ) + break + if weights is None: weights = [1] * len(traces) @@ -1903,6 +1919,13 @@ def sample_prior_predictive( """ model = modelcontext(model) + if model.potentials: + warnings.warn( + "The effect of Potentials on other parameters is ignored during prior predictive sampling. " + "This is likely to lead to invalid or biased predictive samples.", + UserWarning, + ) + if var_names is None: prior_pred_vars = model.observed_RVs prior_vars = ( From a3f8d0575f785aa02ab1d8decb6024d37ad237b2 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Fri, 15 Jan 2021 12:16:13 +0100 Subject: [PATCH 2/4] Add warning to fast_sample_ppc and unittests --- pymc3/distributions/posterior_predictive.py | 8 +++++ pymc3/tests/test_sampling.py | 36 +++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/pymc3/distributions/posterior_predictive.py b/pymc3/distributions/posterior_predictive.py index 2366077fb0..4d6c1f4596 100644 --- a/pymc3/distributions/posterior_predictive.py +++ b/pymc3/distributions/posterior_predictive.py @@ -222,6 +222,14 @@ def fast_sample_posterior_predictive( model = modelcontext(model) assert model is not None + + if model.potentials: + warnings.warn( + "The effect of Potentials on other parameters is ignored during posterior predictive sampling. " + "This is likely to lead to invalid or biased predictive samples.", + UserWarning, + ) + with model: if keep_size and samples is not None: diff --git a/pymc3/tests/test_sampling.py b/pymc3/tests/test_sampling.py index 0decbbd1c9..07c011dfe3 100644 --- a/pymc3/tests/test_sampling.py +++ b/pymc3/tests/test_sampling.py @@ -722,6 +722,22 @@ def test_variable_type(self): assert ppc["a"].dtype.kind == "f" assert ppc["b"].dtype.kind == "i" + def test_potentials_warning(self): + warning_msg = "The effect of Potentials on other parameters is ignored during" + with pm.Model() as m: + a = pm.Normal("a", 0, 1) + p = pm.Potential("p", a) + obs = pm.Normal("obs", a, 1, observed=5) + trace = pm.sample() + + with pytest.warns(UserWarning, match=warning_msg): + with m: + pm.sample_posterior_predictive(trace, samples=5) + + with pytest.warns(UserWarning, match=warning_msg): + with m: + pm.fast_sample_posterior_predictive(trace, samples=5) + class TestSamplePPCW(SeededTest): def test_sample_posterior_predictive_w(self): @@ -773,6 +789,17 @@ def test_sample_posterior_predictive_w(self): ): pm.sample_posterior_predictive_w([trace_0, trace_2], 100, [model_0, model_2]) + def test_potentials_warning(self): + warning_msg = "The effect of Potentials on other parameters is ignored during" + with pm.Model() as m: + a = pm.Normal("a", 0, 1) + p = pm.Potential("p", a) + obs = pm.Normal("obs", a, 1, observed=5) + trace = pm.sample() + + with pytest.warns(UserWarning, match=warning_msg): + pm.sample_posterior_predictive_w(samples=5, traces=[trace, trace], models=[m, m]) + @pytest.mark.parametrize( "method", @@ -1012,6 +1039,15 @@ def test_bounded_dist(self): prior_trace = pm.sample_prior_predictive(5) assert prior_trace["x"].shape == (5, 3, 1) + def test_potentials_warning(self): + warning_msg = "The effect of Potentials on other parameters is ignored during" + with pm.Model() as m: + a = pm.Normal("a", 0, 1) + p = pm.Potential("p", a) + + with pytest.warns(UserWarning, match=warning_msg): + pm.sample_prior_predictive(samples=5) + class TestSamplePosteriorPredictive: def test_point_list_arg_bug_fspp(self, point_list_arg_bug_fixture): From f98f9c75bb29998162da6b107eeb23c432294f21 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Fri, 15 Jan 2021 12:28:56 +0100 Subject: [PATCH 3/4] Add release note --- RELEASE-NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 6e1f0b46a4..f76d5fddb4 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -33,6 +33,7 @@ It also brings some dreadfully awaited fixes, so be sure to go through the chang - Fixed `MatrixNormal` random method to work with parameters as random variables. (see [#4368](https://github.com/pymc-devs/pymc3/pull/4368)) - Update the `logcdf` method of several continuous distributions to return -inf for invalid parameters and values, and raise an informative error when multiple values cannot be evaluated in a single call. (see [4393](https://github.com/pymc-devs/pymc3/pull/4393)) - Improve numerical stability in `logp` and `logcdf` methods of `ExGaussian` (see [#4407](https://github.com/pymc-devs/pymc3/pull/4407)) +- Issue UserWarning when doing prior or posterior predictive sampling with models containing Potential factors (see [#4419](https://github.com/pymc-devs/pymc3/pull/4419)) ## PyMC3 3.10.0 (7 December 2020) From 51db510f1f3a98462e72f6942b9263dadc99d5b3 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Sat, 16 Jan 2021 09:58:19 +0100 Subject: [PATCH 4/4] Avoid sampling in unittests --- pymc3/tests/test_sampling.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pymc3/tests/test_sampling.py b/pymc3/tests/test_sampling.py index 07c011dfe3..c95ad230cb 100644 --- a/pymc3/tests/test_sampling.py +++ b/pymc3/tests/test_sampling.py @@ -726,16 +726,15 @@ def test_potentials_warning(self): warning_msg = "The effect of Potentials on other parameters is ignored during" with pm.Model() as m: a = pm.Normal("a", 0, 1) - p = pm.Potential("p", a) + p = pm.Potential("p", a + 1) obs = pm.Normal("obs", a, 1, observed=5) - trace = pm.sample() - with pytest.warns(UserWarning, match=warning_msg): - with m: + trace = az.from_dict({"a": np.random.rand(10)}) + with m: + with pytest.warns(UserWarning, match=warning_msg): pm.sample_posterior_predictive(trace, samples=5) - with pytest.warns(UserWarning, match=warning_msg): - with m: + with pytest.warns(UserWarning, match=warning_msg): pm.fast_sample_posterior_predictive(trace, samples=5) @@ -793,10 +792,10 @@ def test_potentials_warning(self): warning_msg = "The effect of Potentials on other parameters is ignored during" with pm.Model() as m: a = pm.Normal("a", 0, 1) - p = pm.Potential("p", a) + p = pm.Potential("p", a + 1) obs = pm.Normal("obs", a, 1, observed=5) - trace = pm.sample() + trace = az.from_dict({"a": np.random.rand(10)}) with pytest.warns(UserWarning, match=warning_msg): pm.sample_posterior_predictive_w(samples=5, traces=[trace, trace], models=[m, m]) @@ -1043,8 +1042,9 @@ def test_potentials_warning(self): warning_msg = "The effect of Potentials on other parameters is ignored during" with pm.Model() as m: a = pm.Normal("a", 0, 1) - p = pm.Potential("p", a) + p = pm.Potential("p", a + 1) + with m: with pytest.warns(UserWarning, match=warning_msg): pm.sample_prior_predictive(samples=5)