Invalid strict evaluation of default argument expressions

[phipsgabler]

I expected something like the following to work, but it doesn't:

julia> @model function betareg(
           X,
           y;
           N = size(X, 1),
           K = size(X, 2),
           a = 0.1,
           b = 0.1
       )
           ...
       end
betareg (generic function with 3 methods)

julia> m = betareg(missing, missing; N = 10, K = 2)
ERROR: MethodError: no method matching size(::Missing, ::Int64)
Closest candidates are:
  size(::Union{QR, LinearAlgebra.QRCompactWY, QRPivoted}, ::Integer) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/qr.jl:557
  size(::Union{Cholesky, CholeskyPivoted}, ::Integer) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/cholesky.jl:443
  size(::Union{Hermitian{T, S}, Symmetric{T, S}} where {T, S}, ::Any) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/symmetric.jl:201
  ...
Stacktrace:
 [1] betareg(X::Missing, y::Missing; N::Int64, K::Int64, a::Float64, b::Float64)
   @ Main ./REPL[54]:1
 [2] top-level scope
   @ REPL[55]:1

The reason is that in the model constructor, the expressions for the default arguments are simply spliced again into the named tuple and thus evaluated "strictly":

julia> @macroexpand @model function betareg(
           X,
           y;
           N = size(X, 1),
           K = size(X, 2),
           a = 0.1,
           b = 0.1
       )
           ...
       end
quote
    function betareg(__model__::Model, __varinfo__::AbstractVarInfo, __context__::DynamicPPL.AbstractContext, X, y, N, K, a, b; )
        ...
    end
    begin
        $(Expr(:meta, :doc))
        function betareg(X, y; N = size(X, 1), K = size(X, 2), a = 0.1, b = 0.1)
            #= REPL[56]:1 =#
            return (Model)(:betareg, betareg, NamedTuple{(:X, :y, :N, :K, :a, :b)}((X, y, N, K, a, b)), NamedTuple{(:N, :K, :a, :b)}((size(X, 1), size(X, 2), 0.1, 0.1)))
        end
    end
end

(See Evaluation scope of default values)

I can think of a hacky ad-hoc fix: instead of storing a named tuple of default argument values, store a named tuple of closures, each taking the previous argument values as inputs.

Other than that, it might be possible to replicate the implementation of normal default values, and generate one more than one constructor function directly.

[devmotion]

More radically, do we need to store the default values at all? How are they used?

[devmotion]

I wonder in particular since now the evaluator and the model constructor are the same function, so maybe some of the motivation for having them is gone?

[phipsgabler]

The real motivation is to have something like constructorof. And IIRC the only usage for this is in the prob macro, in case you don't provide all of the arguments.

Buuuut... with condition fully working, the prob macro should be replaceable by it and logdensity anyways.

[devmotion]

Yes, exactly, this is what I assumed. But now in principle we could construct models from existing models without default values for keyword arguments since model.f(args...) -> model.f(args; defaultkwargs...) returns a new model instance with the default positional and keyword arguments.

[phipsgabler]

Uh, right! So we can go remove it, and fix the prob macro. Another hint that this was the right choice to make.

[devmotion]

Would be nice to retire the prob macro and replace it with a less magic functional API IMO.

[yebai]

Closed in favour of #356