Reinstate log-likelihood transforms

[brandonwillard]

This PR reinstates the general log-likelihood transform framework.

I'm not happy with the current approach, so I'll probably make quite a few changes.

[OriolAbril]

I take we better wait for this before adding pointwise log likelihood conversion in #4489 ?

[brandonwillard]

I take we better wait for this before adding pointwise log likelihood conversion in #4489 ?

Not necessarily. The interface for log-likelihood computations shouldn't change much/at all.

[ricardoV94]

I get this error when sampling from a model with Uniforms

with pm.Model() as m:
    x = pm.Uniform('x', 0, 1)
    y = pm.Uniform('y', 0, x, observed=[0.4, 0.5, 0.7])
    
with m:
    trace = pm.sample()

Traceback (most recent call last):
  File "/home/ricardo/Documents/Projects/pymc3-venv/lib/python3.8/site-packages/IPython/core/interactiveshell.py", line 3418, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-4-14e90d4b3669>", line 2, in <module>
    trace = pm.sample()
  File "/home/ricardo/Documents/Projects/pymc3/pymc3/sampling.py", line 429, in sample
    check_start_vals(model.test_point, model)
  File "/home/ricardo/Documents/Projects/pymc3/pymc3/util.py", line 210, in check_start_vals
    raise KeyError(
KeyError: 'Some start parameters do not appear in the model!\nValid keys are: x, y, but x_interval was supplied'

[brandonwillard]

@ricardoV94, I pushed an update that should fix that issue.

More testing is needed, and I need to figure out the best approach to syncing test values, because, right now, changing the test value of a variable does not change the corresponding transformed variable's test value (e.g. changing m.x.tag.test_value in the example above will not affect m.x.tag.value_var.tag.test_value).

N.B. I have no intention of literally "syncing" the corresponding *.tag.test_values; that would be a terrible idea. Instead, the Model object could provide some means of updating that information for corresponding untransformed/transformed terms.

In general, we need to remove PyMC3's use of *.tag.test_value and institute a distinct set of initial values. That is probably best accomplished through the Model interface, and, although *.tag.test_values may be set to the initial values, *.tag.test_value should not be used within PyMC3; otherwise, we'll never be able to wean the codebase off of its costly and problematic reliance on the test value debugging mechanism.

[ricardoV94]

Sounds like we need an entire method to generate / control initial values.

Before we were using the mode/ mean/ median when provided by the distribution classes as well as the update_start_vals function to deal with transformations.

Should we rethink this approach?

---

Unrelated to this, are we able to get transformed random samples automatically or is the transform only appearing in measure space?

[brandonwillard]

Before we were using the mode/ mean/ median when provided by the distribution classes as well as the update_start_vals function to deal with transformations.

We can continue to do that, but in the context of these new initial values, and not by assigning values to *.tag.test_value directly.

Unrelated to this, are we able to get transformed random samples automatically or is the transform only appearing in measure space?

The current implementation automatically creates transformed variables when they're added to the Model object. This is how it works in v3, but, unlike v3, we don't create new random variables or deterministics for the transformed variables.

Now that we distinguish between random variables that are actually random variables (i.e. can be sampled) and variables that are used as inputs to log-likelihood graphs but correspond to random variables (i.e. in poor mathematical notation P(Y == y), where Y is the former and y is the latter), it doesn't make sense to include them anywhere in the Model object except Model.vars—where all the other log-likelihood input variables are.

Since Model.vars returns the symbolic log-likelihood inputs parameters in v4, and those are the variables we use to create log-likelihood graphs, by changing Model.vars so that it returns the transformed log-likelihood inputs (when applicable/available, of course), we'll end up producing and using the transformed log-likelihoods throughout most of the codebase.

[ricardoV94]

Any reason to drop the dunder at the end of the transformed variable name?

[brandonwillard]

No, no reason. Why was it there in the first place?

[ricardoV94]

Preceeded me. My guess would be to avoid name collisions on the user side.

[ricardoV94]

It would be nice to have a helper method to sample transformed variables as well. e.g., SMC starts by sampling from the prior_predictive and converting the samples to untransformed domain to use as the starting point.

V4 makes it easier to go from sample <- -> logp, and this PR should fix the API for logp <- -> transformed_logp. In addition, I think we should also provide a well integrated API to go from sample <- -> transformed_logp (for a serious lack of better terminology). Otherwise I am afraid we will fall back to more fragile solutions behind bugs like #4484 that fail to consider dynamic constraints on the transformations.

[ricardoV94]

I don't know if you intended to deal with this only later, but the current code is only getting samples for the transformed variables during sampling.

with pm.Model() as m:
    x = pm.Uniform('x', 0, 1)

with m:
    trace = pm.sample(compute_convergence_checks=False)

trace.varnames
# ['x_interval']

[brandonwillard]

I don't know if you intended to deal with this only later, but the current code is only getting samples for the transformed variables during sampling.

Yeah, I'll do that next. It seems like we'll need a better way to track these transformed variables, though, because the whole rv_var.tag.value_var approach isn't great.

Plus, it really won't work well when/if we need to keep track of compiled forward/backward transform graphs. I'm talking specifically about aesara.function-compiled graphs that we would use to transform/untransform values (e.g. tests/initial values, samples values, etc.)

[twiecki]

While we over-rely on test-values I just want to add that they are nice for debugging a model.

[ricardoV94]

Yeah, I'll do that next. It seems like we'll need a better way to track these transformed variables, though, because the whole rv_var.tag.value_var approach isn't great.

Plus, it really won't work well when/if we need to keep track of compiled forward/backward transform graphs. I'm talking specifically about aesara.function-compiled graphs that we would use to transform/untransform values (e.g. tests/initial values, samples values, etc.)

The previous approach wasn't great either, looking for missing variables and checking if the names corresponded to the transformed / untransformed strings.

[brandonwillard]

While we over-rely on test-values I just want to add that they are nice for debugging a model.

We definitely won't get rid of the ability to use test values; we simply won't rely on them within our codebase. We can even set test values for variables (based on these proposed initial values, of course) by default within the codebase.

[twiecki]

Perfect.

…

On Thu, Mar 11, 2021, 19:57 Brandon T. Willard ***@***.***> wrote: While we over-rely on test-values I just want to add that they are nice for debugging a model. We definitely won't get rid of the ability to *use* test values; we simply won't rely on them within our codebase. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4521 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFETGAHTB25KJY6PUN5653TDEABTANCNFSM4Y4UPFLA> .

[brandonwillard]

OK, I've added the un-transformed variables to the trace output.

[brandonwillard]

FYI: There are a lot of important fixes in this PR, so I'm probably going to merge it even if all the tests don't pass yet.