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

Document and move matchingvalue #796

Open
mhauru opened this issue Feb 3, 2025 · 1 comment
Open

Document and move matchingvalue #796

mhauru opened this issue Feb 3, 2025 · 1 comment

Comments

@mhauru
Copy link
Member

mhauru commented Feb 3, 2025

compiler.jl has the function matchingvalue, which does conversion of the value given for the purposes of using it in model arguments. It's non-trivial to read and needs documenting, perhaps a more descriptive name, and maybe should be moved to model.jl.

Need for this originally noted here: https://github.com/TuringLang/DynamicPPL.jl/pull/793/files#r1936343226

@penelopeysm
Copy link
Member

penelopeysm commented Feb 3, 2025

eltype's docstring suggests that it could be cleaned up too:

"""
eltype(vi::AbstractVarInfo, spl::Union{AbstractSampler,SampleFromPrior}
Determine the default `eltype` of the values returned by `vi[spl]`.
!!! warning
This should generally not be called explicitly, as it's only used in
[`matchingvalue`](@ref) to determine the default type to use in place of
type-parameters passed to the model.
This method is considered legacy, and is likely to be deprecated in the future.
"""
function Base.eltype(vi::AbstractVarInfo, spl::Union{AbstractSampler,SampleFromPrior})
T = Base.promote_op(getindex, typeof(vi), typeof(spl))
if T === Union{}
# In this case `getindex(vi, spl)` errors
# Let us throw a more descriptive error message
# Ref https://github.com/TuringLang/Turing.jl/issues/2151
return eltype(vi[spl])
end
return eltype(T)
end

I'm not immediately proposing that I'll do this, but it would be interesting to just delete this method and see what breaks. (Locally, of course.)

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