We currently test logpdf_with_trans for all the same distributions as DistributionsAD.jl does, which means that we quite often have bugs in the adjoint of logpdf defined in DistributionsAD.jl delaying PRs made to Bijectors.jl, e.g. #186.
Back in the day it made sense to test this since the logabsdetjac computation was included in logpdf_with_trans and thus we needed to run through distributions to ensure they were AD-compatible. But these days all but one logpdf_with_trans implementation simply defers everything but the logpdf computation to an implementation of logabsdetjac for a particular Bijector.
This means that if we assume DistributionsAD.jl ensures logpdf to be AD-compat for all distributions, then we only need to test the bijectors, not the bijectors + distributions!
Hence this PR.
And if people aren't thoroughly convinced by this, we can also add tests for logpdf_with_trans in the testing suite of DistributionsAD.jl very easily.
EDIT: I think it would actually be a good idea to add the logpdf_with_trans to DA's testing suite as it also ensures that all the distributions introduced there have a bijector implementation + if there's a bug due to Bijectors.jl, it's generally much easier to fix.
We currently test
logpdf_with_trans
for all the same distributions as DistributionsAD.jl does, which means that we quite often have bugs in the adjoint oflogpdf
defined in DistributionsAD.jl delaying PRs made to Bijectors.jl, e.g. #186.Back in the day it made sense to test this since the
logabsdetjac
computation was included inlogpdf_with_trans
and thus we needed to run through distributions to ensure they were AD-compatible. But these days all but onelogpdf_with_trans
implementation simply defers everything but thelogpdf
computation to an implementation oflogabsdetjac
for a particularBijector
.This means that if we assume DistributionsAD.jl ensures
logpdf
to be AD-compat for all distributions, then we only need to test the bijectors, not the bijectors + distributions!Hence this PR.
And if people aren't thoroughly convinced by this, we can also add tests for
logpdf_with_trans
in the testing suite of DistributionsAD.jl very easily.EDIT: I think it would actually be a good idea to add the
logpdf_with_trans
to DA's testing suite as it also ensures that all the distributions introduced there have abijector
implementation + if there's a bug due to Bijectors.jl, it's generally much easier to fix.