Rename `logpdf_with_trans` to `logpdf_with_jacobian`?

[yebai]

The current function name logpdf_with_trans is a bit unclear. Can we rename it to something more self-explanatory, e.g. logpdf_with_jacobian?

[willtebbutt]

Perhaps logpdf_of_trans? i.e. the logpdf of the transformed random variable?

[yebai]

That sounds good too.

[yebai]

@xukai92 @torfjelde Does Will's proposal make sense to you?

[torfjelde]

I agree with changing it, as I initially got quite confused by the name myself. What confused me was the fact that the logpdf_with_trans is computing the logpdf of the transformed variable using the non-transformed one. From both the current name and the one suggested by @willtebbutt I immediately think it's actually doing logpdf(d, invlink(d, y)) + logabsdetjac(invlink, y) rather than logpdf(d, x) - logabsdetjac(link, x), where y is the transformed variable and x = rand(d), if that makes sense. It's of course the same result, but what's the expected input was kind of confusing to me.

With that being said, I realize such changing the internals of the methods might be a bit drastic given other packages' dependence on Bijectors.jl. So in that case, I think I prefer logpdf_with_jac or logpdf_with_jacobian, as it doesn't give such a strong indication that the expected input is the transformed variable when it isn't.

Worth noting that I already introduced logpdf_with_jac in the recent interface PR, which returns a tuple (logpdf_with_trans, logjac). But we also recently introduced a forward(d::Distribution) which kind of replaces logpdf_with_jac and is more useful, so we could easily drop that method and use the name for this purpose instead.

[xukai92]

I prefer logpdf_with_jac.

Also compared with forward, logpdf_with_jac is more verbose - when people reads the code they at least know it's doing something about computing the log density with some Jacobian computation invovled.

[yebai]

logpdf_with_jac looks good to me!