TuringLang / Bijectors.jl

Implementation of normalising flows and constrained random variable transformations
https://turinglang.org/Bijectors.jl/
MIT License
200 stars 33 forks source link

Incorrect bijector for heterogeneous Product distribution #290

Closed bgroenks96 closed 11 months ago

bgroenks96 commented 11 months ago

As noted in the code, the implementation of bijector for heterogeneous product distributions doesn't make sense. Why is not just Stacked(map(bijector, d.v))?

Here is my fixed version:

function Bijectors.bijector(prod::Product{Continuous})
    D = eltype(prod.v)
    return if Bijectors.has_constant_bijector(D)
        Bijectors.elementwise(bijector(prod.v[1]))
    else
        Bijectors.Stacked(map(bijector, Tuple(prod.v)))
    end
end
torfjelde commented 11 months ago

I believe I did this because the previous version was using the TruncatedBijector and so I didn't want to touch in the PR where I added has_constant_bijector, etc.

The downside with the map approach is that it will be much slower vs. vectorized TruncatedBijector (and UnivariateDistribution should be implementing the minimum and maximum anyways, no?)

bgroenks96 commented 11 months ago

I can check again, but I don't think it works when the array of distributions is heterogeneous.

bgroenks96 commented 11 months ago

Ok, it actually seems to work... I can't reproduce the behavior I saw before. I'll just assume I did something wrong!