Bijector not defined for FiniteGP about stheno.jl HOT 13 CLOSED

Sweet! I'll make a PR for Bijectors.jl 👍

from stheno.jl.

For reference, running the following (copied from here and here) before running the Turing model fixes the error:

Bijectors.bijector(d::Stheno.FiniteGP) = Identity{1}()
for T in (:VectorOfMultivariate, :FillVectorOfMultivariate)
    @eval begin
        Bijectors.bijector(d::Bijectors.$T{Continuous, <:Stheno.FiniteGP}) = Identity{2}()
    end
end

from stheno.jl.

Thanks for opening this. I'll try and take a look soon.

from stheno.jl.

Ran into a similar problem trying to use a logit link and bernoulli likelihood function. Is the above still the best solution to this?

from stheno.jl.

Hmm I guess.

@torfjelde do you know what's going on here?

from stheno.jl.

Yeah, I'd say so. Not too long ago we removed the "default"-implementations, which was to just use Identity if we didn't recognize the distribution. Unfortunately that lead to a lot of silent bugs, hence we now require explicit implementation of bijector. And the above solution by @ElOceanografo is good:)

EDIT: Ideally that should just be made part of the package implementing the distribution, but we're probably not quite there yet for Bijectors.jl.

from stheno.jl.

Ah I see. @torfjelde what's the correct place to solve this? Bijectors has quite a lot of deps that I'm not keen to take on in Stheno, so I would prefer not to have to use it here just to solve Turing integration.

from stheno.jl.

Well, I guess that kind means that we haven't done our job correctly, haha. The idea was for Bijectors.jl to be lightweight so that anyone who's using Distributions.jl wouldn't have any issues also depending on Bijectors.jl.

Are there any particular dependencies that makes this an issue?

With the exception of Roots, NNlib and MappedArrays I think we have essentially the same deps as Distributions.jl, no?

from stheno.jl.

Oh interesting. I actually hadn't noticed that / hadn't realised how lightweight all of those packages are.

I think I'm probably okay depending on Bijectors then. Could someone @torfjelde @ElOceanografo make a PR to Stheno to incorporate this?

A good place to add this would be another file in the util directory. Should be included below here in Stheno.jl:
https://github.com/willtebbutt/Stheno.jl/blob/f0f78aae68122d01b582296fea186715ecf88a96/src/Stheno.jl#L65

from stheno.jl.

Although, actually, @torfjelde is bijector defined for AbstractMvNormal, or just MvNormal? If it's the former then the FiniteGP could just subtype AbstractMvNormal instead of ContinuousMultivariateDistribution.

from stheno.jl.

Ah, awesome!:)

And we're currently implementing it for MvNormal, but don't see any reason why we shouldn't do it for AbstractMvNormal, so we could at least make that change on Bijectors.jl's end 👍

from stheno.jl.

Fantastic. I'll change things on my end, then hopefully it will "just work".

from stheno.jl.

Will leave this open until we've got new versions tagged.

from stheno.jl.