Repository: pyro-ppl/numpyro

Issue:
saving render_model() output to the desired file path
When calling `numpyro.render_model(model, filename=my_path)` I was expecting the generated file to be saved in `my_path`. But it does not. 

Reading the source code, it seems that it deliberately use `filename.stem` instead of user provided file path. Is this the intended behavior? Unable to save the file to the path user requested make the filename option useless. 

```python
def render_model(
    model,
    model_args=None,
    model_kwargs=None,
    filename=None,
    render_distributions=False,
    render_params=False,
):
    """
    Wrap all functions needed to automatically render a model.

    .. warning:: This utility does not support the
        :func:`~numpyro.contrib.control_flow.scan` primitive.
        If you want to render a time-series model, you can try
        to rewrite the code using Python for loop.

    :param model: Model to render.
    :param model_args: Positional arguments to pass to the model.
    :param model_kwargs: Keyword arguments to pass to the model.
    :param str filename: File to save rendered model in.
    :param bool render_distributions: Whether to include RV distribution annotations in the plot.
    :param bool render_params: Whether to show params in the plot.
    """
    relations = get_model_relations(
        model,
        model_args=model_args,
        model_kwargs=model_kwargs,
    )
    graph_spec = generate_graph_specification(relations, render_params=render_params)
    graph = render_graph(graph_spec, render_distributions=render_distributions)

    if filename is not None:
        filename = Path(filename)
        graph.render(
            filename.stem, view=False, cleanup=True, format=filename.suffix[1:]
        )  # remove leading period from suffix

    return graph
```

Discussion:

Patch:
diff --git a/notebooks/source/bayesian_hierarchical_linear_regression.ipynb b/notebooks/source/bayesian_hierarchical_linear_regression.ipynb
index cfa800d0..cbf44ff1 100644
--- a/notebooks/source/bayesian_hierarchical_linear_regression.ipynb
+++ b/notebooks/source/bayesian_hierarchical_linear_regression.ipynb
@@ -427,6 +427,13 @@
     "samples_predictive = predictive(random.PRNGKey(0), patient_code, Weeks, None)"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "Note that for [`Predictive`](http://num.pyro.ai/en/latest/utilities.html#numpyro.infer.util.Predictive) to work as expected, the response variable of the model (in this case, `FVC_obs`) must be set to `None`."
+   ]
+  },
   {
    "cell_type": "markdown",
    "metadata": {},
diff --git a/notebooks/source/bayesian_regression.ipynb b/notebooks/source/bayesian_regression.ipynb
index d798f9a4..aaf82160 100644
--- a/notebooks/source/bayesian_regression.ipynb
+++ b/notebooks/source/bayesian_regression.ipynb
@@ -1507,6 +1507,14 @@
     "ax.set(xlabel=\"Marriage rate\", ylabel=\"Divorce rate\", title=\"Predictions with 90% CI\");"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "Note that for `Predictive` to work as expected, the response variable of the model (in this case, `divorce`) must be set to `None`.\n",
+    "In the code above this is done implicitly by not passing a value for `divorce` to the model in the call to `prior_predictive`, which due to the model definition, sets `divorce=None`."
+   ]
+  },
   {
    "cell_type": "markdown",
    "metadata": {
diff --git a/numpyro/distributions/util.py b/numpyro/distributions/util.py
index 7b0e2632..d32d2da2 100644
--- a/numpyro/distributions/util.py
+++ b/numpyro/distributions/util.py
@@ -149,6 +149,8 @@ def _binomial_inversion(key, p, n):
         return cond_exclude_large_mu & (geom_acc <= n)
 
     log1_p = jnp.log1p(-p)
+    # Make sure p=0 is never taken into account as a fix for possible zeros in p.
+    log1_p = jnp.where(log1_p == 0, -jnp.finfo(log1_p.dtype).tiny, log1_p)
     ret = lax.while_loop(_binom_inv_cond_fn, _binom_inv_body_fn, (-1, key, 0.0))
     return ret[0]
 
diff --git a/numpyro/infer/inspect.py b/numpyro/infer/inspect.py
index abfb9bd7..6b8d5d05 100644
--- a/numpyro/infer/inspect.py
+++ b/numpyro/infer/inspect.py
@@ -629,9 +629,14 @@ def render_model(
 
     if filename is not None:
         filename = Path(filename)
+        # remove leading period from suffix
+        filename_without_suffix = filename.with_suffix("")
         graph.render(
-            filename.stem, view=False, cleanup=True, format=filename.suffix[1:]
-        )  # remove leading period from suffix
+            filename_without_suffix,
+            view=False,
+            cleanup=True,
+            format=filename.suffix[1:],
+        )
 
     return graph
 
diff --git a/numpyro/infer/reparam.py b/numpyro/infer/reparam.py
index 60be9fa7..f191100c 100644
--- a/numpyro/infer/reparam.py
+++ b/numpyro/infer/reparam.py
@@ -338,6 +338,7 @@ class CircularReparam(Reparam):
         if isinstance(support, constraints.independent):
             support = fn.support.base_constraint
         assert support is constraints.circular
+        assert obs is None, "CircularReparam does not support observe statements"
 
         # Draw parameter-free noise.
         new_fn = dist.ImproperUniform(constraints.real, fn.batch_shape, fn.event_shape)
diff --git a/numpyro/infer/util.py b/numpyro/infer/util.py
index 56c4d2c4..b0c16a08 100644
--- a/numpyro/infer/util.py
+++ b/numpyro/infer/util.py
@@ -857,6 +857,9 @@ class Predictive(object):
         The interface for the `Predictive` class is experimental, and
         might change in the future.
 
+    Note that for the predictive distribution to be returned as intended, observed
+    variables in the model (constraining the likelihood term) must be set to `None` (see Example).
+
     :param model: Python callable containing Pyro primitives.
     :param dict posterior_samples: dictionary of samples from the posterior.
     :param callable guide: optional guide to get posterior samples of sites not present
@@ -908,6 +911,10 @@ class Predictive(object):
         predictive = Predictive(model, num_samples=1000)
         y_pred = predictive(rng_key, X)["obs"]
 
+    Note how above, no value for `y` is passed to `predictive`, resulting in `y`
+    being set to `None`. Setting the observed variable(s) to `None` when using
+    `Predictive` is required for the method to function as expected.
+
     If you also have posterior samples, you can sample from the posterior predictive::
 
         predictive = Predictive(model, posterior_samples=posterior_samples)


Test patch:
diff --git a/test/infer/test_reparam.py b/test/infer/test_reparam.py
index f1d72f74..e734f3c9 100644
--- a/test/infer/test_reparam.py
+++ b/test/infer/test_reparam.py
@@ -378,6 +378,16 @@ def test_circular(shape):
     assert_allclose(actual_probe, expected_probe, atol=0.1)
 
 
+def test_circular_reparam_no_observe():
+    def model():
+        numpyro.sample("x", dist.VonMises(0, 1), obs=0.5)
+
+    with numpyro.handlers.seed(rng_seed=0):
+        with numpyro.handlers.reparam(config={"x": CircularReparam()}):
+            with pytest.raises(AssertionError, match="not support observe"):
+                model()
+
+
 _unconstrain_reparam = numpyro.infer.util._unconstrain_reparam
 
 
diff --git a/test/test_model_rendering.py b/test/test_model_rendering.py
index 62b7cf65..a543add9 100644
--- a/test/test_model_rendering.py
+++ b/test/test_model_rendering.py
@@ -1,6 +1,8 @@
 # Copyright Contributors to the Pyro project.
 # SPDX-License-Identifier: Apache-2.0
 
+import os
+
 import numpy as np
 import pytest
 
@@ -8,7 +10,11 @@ import jax.numpy as jnp
 
 import numpyro
 import numpyro.distributions as dist
-from numpyro.infer.inspect import generate_graph_specification, get_model_relations
+from numpyro.infer.inspect import (
+    generate_graph_specification,
+    get_model_relations,
+    render_model,
+)
 
 
 def simple(data):
@@ -129,3 +135,12 @@ def test_model_transformation(test_model, model_kwargs, expected_graph_spec):
     graph_spec = generate_graph_specification(relations)
 
     assert graph_spec == expected_graph_spec
+
+
+def test_render_model_filename():
+    def model():
+        numpyro.sample("x", dist.Normal(0, 1))
+
+    render_model(model, filename="graph.png")
+    assert os.path.exists("graph.png")
+    os.remove("graph.png")