Skip to content

Broken init for MatrixDistribution and CholeskyVariate with multiple samples #493

@harisorgn

Description

@harisorgn

init has been updated mainly in #489 and then in #485 to match recent changes in Bijectors.jl .Do we need to update the multisample methods

DynamicPPL.jl/src/utils.jl

Lines 311 to 324 in e6dd4ef

################################
# Multi-sample initialisations #
################################
function inittrans(rng, dist::UnivariateDistribution, n::Int)
return Bijectors.invlink(dist, randrealuni(rng, n))
end
function inittrans(rng, dist::MultivariateDistribution, n::Int)
return Bijectors.invlink(dist, randrealuni(rng, size(dist)[1], n))
end
function inittrans(rng, dist::MatrixDistribution, n::Int)
return Bijectors.invlink(dist, [randrealuni(rng, size(dist)...) for _ in 1:n])
end
as well?

This is mainly for MatrixDistribution and Distribution{CholeskyVariate} which currently lead to StackOverFlow with multisample init. That's because calls keep bouncing in generic Bijectors.transform. One straightforward fix would be

function inittrans(rng, dist::MatrixDistribution, n::Int)
    # Get the size of the unconstrained vector
    b = link_transform(dist)
    sz = Bijectors.output_size(b, size(dist))
    return Bijectors.invlink.(Ref(dist), [randrealuni(rng, sz...) for _ in 1:n])
end
function inittrans(rng, dist::Distribution{<:CholeskyVariate}, n::Int)
    # Get the size of the unconstrained vector
    b = link_transform(dist)
    sz = Bijectors.output_size(b, size(dist))
    return Bijectors.invlink.(Ref(dist), [randrealuni(rng, sz...) for _ in 1:n])
end

But are these methods ever needed? IIUC only MultiVariateDistribution uses the multisample init in assume. Just putting this out there. cc @torfjelde

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions