Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logpdf of UnivariateTransformed produces StackOverflowError #251

Open
lgrozinger opened this issue Mar 24, 2023 · 7 comments
Open

logpdf of UnivariateTransformed produces StackOverflowError #251

lgrozinger opened this issue Mar 24, 2023 · 7 comments

Comments

@lgrozinger
Copy link

lgrozinger commented Mar 24, 2023

Taking the logpdf of a MultivariateTransformed works.

julia> td = transformed(MvNormal(zeros(2), ones(2)), PlanarLayer(2))
MultivariateTransformed{DiagNormal, PlanarLayer{Vector{Float64}, Vector{Float64}}}(
dist: DiagNormal(
dim: 2
μ: [0.0, 0.0]
Σ: [1.0 0.0; 0.0 1.0]
)

transform: PlanarLayer(w = [-0.18499400975237437, 0.7807898767400905], u = [-0.8954510142425457, 0.6844620924181255], b = [0.0004760280592709614])
)

julia> logpdf(td, [1, 1])
-3.356783325529313

This is great! All hail the developers! But for a UnivariateTransformed it does not work.

julia> td = transformed(Normal(), flow)
UnivariateTransformed{Normal{Float64}, PlanarLayer{Vector{Float64}, Vector{Float64}}}(
dist: Normal{Float64}=0.0, σ=1.0)
transform: PlanarLayer(w = [0.16965000037825323], u = [-0.7102609218532135], b = [2.6289813900673975])
)

julia> logpdf(td, 1)
ERROR: StackOverflowError:
Stacktrace:
     [1] transform(t::Inverse{PlanarLayer{Vector{Float64}, Vector{Float64}}}, x::Int64)
       @ Bijectors ~/.julia/packages/Bijectors/vKGbw/src/interface.jl:60
     [2] with_logabsdet_jacobian(ib::Inverse{PlanarLayer{Vector{Float64}, Vector{Float64}}}, y::
Int64)                                                                                         
       @ Bijectors ~/.julia/packages/Bijectors/vKGbw/src/interface.jl:177
--- the last 2 lines are repeated 39990 more times ---
 [79983] transform(t::Inverse{PlanarLayer{Vector{Float64}, Vector{Float64}}}, x::Int64)
       @ Bijectors ~/.julia/packages/Bijectors/vKGbw/src/interface.jl:60

I understand that most use cases for this package will want MultivariateTransformed, but I would have expected this to work. Either I'm missing something or this is a bug. Can anyone confirm either way?

I am using Bijectors v0.12.3.

@torfjelde
Copy link
Member

All hail the developers!

Much appreciated 🙏

But for a UnivariateTransformed it does not work.

Unfortunately it doesn't work because the PlanarLayer works with vectors, not Real, i.e. this particular pair of distribution and transformation is not compatible. The resulting error message is not particularly informative, but this is due to some useful default implementations.

@lgrozinger
Copy link
Author

Unfortunately it doesn't work because the PlanarLayer works with vectors, not Real, i.e. this particular pair of distribution and transformation is not compatible.

Hmmm ok, but I assume you mean that the Bijectors.PlanarLayer works only with vectors, not the mathematical object? Maybe I'm wrong, but the definition in Variational Inference with Normalizing Flows doesn't mention anything about restricting to only vectors?

@lgrozinger
Copy link
Author

lgrozinger commented Mar 24, 2023

The resulting error message is not particularly informative, but this is due to some useful default implementations.

If it really cannot be done, perhaps we could return a useful error message when someone tries PlanarLayer(1)?

@torfjelde
Copy link
Member

Hmmm ok, but I assume you mean that the Bijectors.PlanarLayer works only with vectors, not the mathematical object?

Yeah, the actual transformation does work with a real IIRC.

If it really cannot be done, perhaps we could return a useful error message when someone tries PlanarLayer(1)?

It can be done, but the current implementation assumes we're working with an array and so we'd need to implement something that works with a Real.

IMO this doesn't quite seem worth it since you can always just do PlanarLayer(1) and then wrap your x::Real as an array, i.e. [x].

That's not sufficient because PlanarLayer(1) means "we're going to work with input AbstractVector of length 1" which is different from working with a Real.

I agree we should error if you try to call PlanarLayer on a Real though; this is definitively something we should add:)

@torfjelde
Copy link
Member

So for now, instead of doing transformed(Normal(), ...), just do transformed(MvNormal(zeros(1), LinearAlgebra.I), ...).

@lgrozinger
Copy link
Author

Ok thank you for your help.

Perhaps when I have a bit of time I can get to grips with the codebase and propose a fix.

@torfjelde
Copy link
Member

That would be very much appreciated @lgrozinger ! Will keep an eye out:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants