search

TuringLang / Bijectors.jl

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

Throw error if `ordered` unsafe to use #241

Closed sethaxen closed 1 year ago

sethaxen commented 1 year ago

As noted in https://github.com/TuringLang/Bijectors.jl/issues/220#issuecomment-1409115410, ordered(d) does the incorrect thing unless d is unconstrained. This PR adds a check that throws an error if this is not the case. Note that examples like this will now error even though they are fine to use with ordered, since they don't return a Identity bijector:

julia> bijector(Product(fill(Normal(), 5)))
Bijectors.TruncatedBijector{1, Float64, Float64}(-Inf, Inf)

julia> bijector(Product([Normal(), Cauchy()]))
Bijectors.TruncatedBijector{1, Vector{Float64}, Vector{Float64}}([-Inf, -Inf], [Inf, Inf])
rikhuijzer commented 1 year ago

Uhm for the next time, this should have been a breaking release, no? There could be clients running production depending on the old and broken implementation. They now have to adjust or have no output at all, which means this change is breaking.

devmotion commented 1 year ago

depending on the old and broken implementation.

According to semver, bug fixes are non-breaking. The only problem I see here are the now-failing examples in the OP - but it seems they could be fixed by improved definitions of bijector for product distributions or an improved check that also supports TruncatedBijectors with infinite bounds.