Skip to content

getindex(vi::AbstractVarInfo, ::Colon) docstring is wrong #854

Open
@penelopeysm

Description

@penelopeysm

"""
getindex(vi::AbstractVarInfo, ::Colon)
Return the current value(s) of `vn` (`vns`) in `vi` in the support of its (their)
distribution(s) as a flattened `Vector`.

This is clearly not true, we regularly use vi[:] to access a vector of params as stored internally.

julia> using DynamicPPL, Distributions

julia> @model f() = x ~ Beta()
f (generic function with 2 methods)

julia> m = f(); vi = VarInfo(m); vi2 = link(vi, m);

julia> vi[:]
1-element Vector{Float64}:
 0.4059923090008866

julia> vi2[:]
1-element Vector{Float64}:
 -0.3805580510140428

Note that the default implementation

Base.getindex(vi::AbstractVarInfo, ::Colon) = values_as(vi, Vector)

just calls getindex_internal:

values_as(vi::VarInfo, ::Type{Vector}) = copy(getall(vi))

getall(vi::TypedVarInfo) = reduce(vcat, map(getall, vi.metadata))
function getall(md::Metadata)
return mapreduce(
Base.Fix1(getindex_internal, md), vcat, md.vns; init=similar(md.vals, 0)
)
end

In theory, the simplest fix to this would be to change the docstring to say that it returns possibly-transformed values.

However, it is technically inconsistent that vi[:] gives internal storage (which may be transformed) whereas vi[@varname(x)] will give you the untransformed values. The correct solution should be to fix getindex(vi, :) to give the untransformed values, and any time we need the transformed values, we should be using getindex_internal(vi, :). This will be pretty annoying, because the 'abuse' of vi[:] is everywhere in the codebase.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions