JuliaStaging / JuliaStaging/GeneralizedGenerated.jl

Is Box.Core too aggressive?

Open
#72 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Julia
Stars
89
Forks
12
PR merge metrics
No merged PRs in 30d

Description

Hi, I've been able to get a little more detail on an issue I've been seeing in Soss and Tilde. As usual, it's entirely possible the bug may be in one of my packages, but I hope this example might help narrow down the problem.

Say we have this in Tilde:
```julia
m = @model n begin
p ~ Uniform()
x ~ For(n) do j
Bernoulli(p)
end
end

x = rand(Bool, 3)
logdensityof(m(3) | (;x), (p=0.2,))
```
From that, Tilde produces this code:
```julia
:(@inline function (_mc, _cfg, _ctx, _pars, _retfun)
local _retn
_args = (Tilde.argvals)(_mc)
_obs = (Tilde.observations)(_mc)
_cfg = merge(_cfg, (args = _args, obs = _obs, pars = _pars))
n::Int64 = _args.n
x::Vector{Bool} = _obs.x
p::Float64 = _pars.p
(p, _ctx, _retn) = (Tilde.tilde)(DensityInterface.logdensityof, (Accessors.opticcompose)(), static(:p), (Tilde.Unobserved)(p), Uniform(), _cfg, _ctx)
_retn isa Tilde.ReturnNow && return _retn.value
(x, _ctx, _retn) = (Tilde.tilde)(DensityInterface.logdensityof, (Accessors.opticcompose)(), static(:x), (Tilde.Observed)(x), For(function (j,)
Bernoulli(p)
end, n), _cfg, _ctx)
_retn isa Tilde.ReturnNow && return _retn.value
return (Tilde.var"#61#63"())(_ctx, _ctx)
end)
```

In this code, `p` is declared `local`, then assigned, and then used in a closure. But it should be easy to guarantee that `p` does not change its type. We usually have this, which is why I have lines like ` p::Float64 = _pars.p`. I got this from [the Julia docs](https://docs.julialang.org/en/v1/manual/performance-tips/#man-performance-captured), which say
> _If captured variables are used in a performance-critical section of the code, then the following tips help ensure that their use is performant. First, if it is known that a captured variable does not change its type, then this can be declared explicitly with a type annotation (on the variable, not the right-hand side)_

Anyway, calling this expression `q`, we then call
```julia
q = from_type(_get_gg_func_body(mk_function(M, q))) |> MacroTools.flatten
```
which yields
```julia

quote
p = Core.Box()
$(Expr(:meta, :((Main).inline)))
local _retn
_args = (Tilde.argvals)(_mc)
_obs = (Tilde.observations)(_mc)
_cfg = (Main).merge(_cfg, (args = _args, obs = _obs, pars = _pars))
n::Int64 = _args.n
x::Vector{Bool} = _obs.x
p.contents::Float64 = _pars.p
(p.contents, _ctx, _retn) = (Tilde.tilde)(DensityInterface.logdensityof, (Accessors.opticcompose)(), static(:p), (Tilde.Unobserved)(p.contents), (Main).Uniform(), _cfg, _ctx)
(Main).isa(_retn, (Main).Tilde.ReturnNow) && return _retn.value
(x, _ctx, _retn) = (Tilde.tilde)(DensityInterface.logdensityof, (Accessors.opticcompose)(), static(:x), (Tilde.Observed)(x), (Main).For(begin
let freevars = (p,)
(GeneralizedGenerated.Closure){function = (p, j;) -> begin
begin
(Main).Bernoulli(p.contents)
end
end, Base.typeof(freevars)}(freevars)
end
end, n), _cfg, _ctx)
(Main).isa(_retn, (Main).Tilde.ReturnNow) && return _retn.value
return (Tilde.var"#61#63"())(_ctx, _ctx)
end
```

This ends up boxing `p`. But that's not needed - working with a modified GG I can just remove the lines that create the box, and it works great.

The boxing leads to problems, because the assignment `p.contents::Float64 = _pars.p` is not valid.

Do you see any changes I could make to my codegen to correct for this, or it does this require a change in GG?

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.