From 401f8e49119390630f7ddd41a4fba40395c9085f Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Sat, 17 Jul 2021 19:13:47 +0200 Subject: [PATCH 01/10] Fix begin/end replacement in varname expansion (fixes #23) --- src/varname.jl | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 92d17133..39b4946a 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -224,6 +224,10 @@ A macro that returns an instance of [`VarName`](@ref) given a symbol or indexing The `sym` value is taken from the actual variable name, and the index values are put appropriately into the constructor (and resolved at runtime). +NB: `begin` and `end` indexing can be used, but remember that they depend on _runtime values_ -- +their usage requires that the array over which the indexing expression is defined is defined, in +order for `firstindex` and `lastindex` to work in the expanded code. + ## Examples ```jldoctest @@ -241,6 +245,9 @@ julia> @varname(x[:, 1][2]).indexing julia> @varname(x[1,2][1+5][45][3]).indexing ((1, 2), (6,), (45,), (3,)) + +julia> let a = [42]; @varname(a[1][end][3]); end +a[1][1][3] ``` !!! compat "Julia 1.5" @@ -334,6 +341,14 @@ macro vinds(expr::Union{Expr, Symbol}) end +@static if VERSION < v"1.5.0-DEV.666" + _replace_ref_begin_end(ex, withex) = Base.replace_ref_end_!(copy(ex), withex)[1] + _index_replacement_for(s) = :($lastindex($s)) +else + _replace_ref_begin_end(ex, withex) = Base.replace_ref_begin_end_!(copy(ex), withex)[1] + _index_replacement_for(s) = :($firstindex($s)), :($lastindex($s)) +end + """ vinds(expr) @@ -348,23 +363,30 @@ julia> vinds(:(x[end])) julia> vinds(:(x[1, end])) :(((1, (lastindex)(x, 2)),)) + +julia> vinds(:(x[1][end])) +:(((1,), ((lastindex)(x),))) + +julia> vinds(:(x[1][2, end, :][3])) +:(((1,), (2, (lastindex)(x), :), (3,))) ``` """ -function vinds end +function vinds(expr) + index_replacement = _index_replacement_for(vsym(expr)) + bare_vinds = _vinds(expr) + return _replace_ref_begin_end(bare_vinds, index_replacement) +end -vinds(expr::Symbol) = Expr(:tuple) -function vinds(expr::Expr) +_vinds(expr::Symbol) = Expr(:tuple) +function _vinds(expr::Expr) if Meta.isexpr(expr, :ref) ex = copy(expr) - @static if VERSION < v"1.5.0-DEV.666" - Base.replace_ref_end!(ex) - else - Base.replace_ref_begin_end!(ex) - end + init = _vinds(ex.args[1]).args last = Expr(:tuple, ex.args[2:end]...) - init = vinds(ex.args[1]).args return Expr(:tuple, init..., last) else error("Mis-formed variable name $(expr)!") end end + + From 06f29985b4f06e27fa9275c46e8fec5b7a2068ee Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Sun, 18 Jul 2021 13:29:37 +0200 Subject: [PATCH 02/10] More complicated reimplementation, partial caching --- src/varname.jl | 65 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 39b4946a..95a28813 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -342,10 +342,12 @@ end @static if VERSION < v"1.5.0-DEV.666" - _replace_ref_begin_end(ex, withex) = Base.replace_ref_end_!(copy(ex), withex)[1] + _replace_ref_begin_end(ex, withex) = Base.replace_ref_end_!(copy(ex), withex) + _replace_ref_begin_end(ex::Symbol, withex) = Base.replace_ref_end_!(ex, withex) _index_replacement_for(s) = :($lastindex($s)) else - _replace_ref_begin_end(ex, withex) = Base.replace_ref_begin_end_!(copy(ex), withex)[1] + _replace_ref_begin_end(ex, withex) = Base.replace_ref_begin_end_!(copy(ex), withex) + _replace_ref_begin_end(ex::Symbol, withex) = Base.replace_ref_begin_end_!(ex, withex) _index_replacement_for(s) = :($firstindex($s)), :($lastindex($s)) end @@ -372,21 +374,62 @@ julia> vinds(:(x[1][2, end, :][3])) ``` """ function vinds(expr) - index_replacement = _index_replacement_for(vsym(expr)) - bare_vinds = _vinds(expr) - return _replace_ref_begin_end(bare_vinds, index_replacement) + indexing = _straighten_indexing(expr) + head = vsym(expr) + + cached_exprs = Vector{Pair{Symbol, Expr}}() + + # see https://github.com/JuliaLang/julia/blob/bb5b98e72a151c41471d8cc14cacb495d647fb7f/base/views.jl#L17-L75 + inds, _ = foldl(indexing, init=(Expr[], head)) do (inds, partial), ixs + S = (partial == head) ? head : gensym(:S) + used_S = false + + nixs = length(ixs) + if nixs == 1 + ixs[1], used_S = _replace_ref_begin_end( + ixs[1], + (:($firstindex($S)), :($lastindex($S))) + ) + elseif nixs > 1 + for i in eachindex(ixs) + ixs[i], used = _replace_ref_begin_end( + ixs[i], + (:($firstindex($S, $i)), :($lastindex($S, $i))) + ) + used_S |= used + end + end + + if used_S && partial !== head + # partial = Expr(:ref, S, ixs...) + push!(cached_exprs, S => partial) + # else + end + partial = Expr(:ref, partial, ixs...) + + + inds = push!(inds, Expr(:tuple, ixs...)) + inds, partial + end + + tuple_expr = Expr(:tuple, inds...) + cached_assignments = [:($S = $partial) for (S, partial) in cached_exprs] + return Expr(:let, Expr(:block, cached_assignments...), tuple_expr) end -_vinds(expr::Symbol) = Expr(:tuple) -function _vinds(expr::Expr) +_straighten_indexing(expr::Symbol) = Vector{Any}[] +function _straighten_indexing(expr::Expr) if Meta.isexpr(expr, :ref) - ex = copy(expr) - init = _vinds(ex.args[1]).args - last = Expr(:tuple, ex.args[2:end]...) - return Expr(:tuple, init..., last) + init = _straighten_indexing(expr.args[1]) + last = expr.args[2:end] + return push!(init, last) else error("Mis-formed variable name $(expr)!") end end + + + + From a278b5dab1e2156beecae3fac3677dd139dac3b9 Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Mon, 19 Jul 2021 12:23:26 +0200 Subject: [PATCH 03/10] Try to fix caching --- src/varname.jl | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 95a28813..244c30d1 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -261,8 +261,9 @@ end varname(sym::Symbol) = :($(AbstractPPL.VarName){$(QuoteNode(sym))}()) function varname(expr::Expr) if Meta.isexpr(expr, :ref) - sym, inds = vsym(expr), vinds(expr) - return :($(AbstractPPL.VarName){$(QuoteNode(sym))}($inds)) + head = vsym(expr) + inds = vinds(expr, head) + return :($(AbstractPPL.VarName){$(QuoteNode(head))}($inds)) else error("Malformed variable name $(expr)!") end @@ -373,10 +374,8 @@ julia> vinds(:(x[1][2, end, :][3])) :(((1,), (2, (lastindex)(x), :), (3,))) ``` """ -function vinds(expr) +function vinds(expr, head = vsym(expr)) indexing = _straighten_indexing(expr) - head = vsym(expr) - cached_exprs = Vector{Pair{Symbol, Expr}}() # see https://github.com/JuliaLang/julia/blob/bb5b98e72a151c41471d8cc14cacb495d647fb7f/base/views.jl#L17-L75 @@ -386,10 +385,11 @@ function vinds(expr) nixs = length(ixs) if nixs == 1 - ixs[1], used_S = _replace_ref_begin_end( + ixs[1], used = _replace_ref_begin_end( ixs[1], (:($firstindex($S)), :($lastindex($S))) ) + used_S |= used elseif nixs > 1 for i in eachindex(ixs) ixs[i], used = _replace_ref_begin_end( @@ -401,12 +401,14 @@ function vinds(expr) end if used_S && partial !== head - # partial = Expr(:ref, S, ixs...) push!(cached_exprs, S => partial) - # else end - partial = Expr(:ref, partial, ixs...) - + + if length(cached_exprs) > 0 && cached_exprs[end][2] == partial + partial = Expr(:ref, cached_exprs[end][1], ixs...) + else + partial = Expr(:ref, partial, ixs...) + end inds = push!(inds, Expr(:tuple, ixs...)) inds, partial From db991c2f178f492d08acbe9b6d078b8567d31fa4 Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Mon, 19 Jul 2021 12:29:09 +0200 Subject: [PATCH 04/10] Convert foldl to a loop; update doctests --- src/varname.jl | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 244c30d1..b2685c20 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -368,18 +368,24 @@ julia> vinds(:(x[1, end])) :(((1, (lastindex)(x, 2)),)) julia> vinds(:(x[1][end])) -:(((1,), ((lastindex)(x),))) +:(let var"##S#321" = x[1] + ((1,), ((lastindex)(var"##S#321"),)) + end) julia> vinds(:(x[1][2, end, :][3])) -:(((1,), (2, (lastindex)(x), :), (3,))) +:(let var"##S#322" = x[1] + ((1,), (2, (lastindex)(var"##S#322", 2), :), (3,)) + end) ``` """ function vinds(expr, head = vsym(expr)) + # see https://github.com/JuliaLang/julia/blob/bb5b98e72a151c41471d8cc14cacb495d647fb7f/base/views.jl#L17-L75 indexing = _straighten_indexing(expr) - cached_exprs = Vector{Pair{Symbol, Expr}}() + inds = Expr[] # collection of result indices + partial = head # partial :ref expressions, used in caching + cached_exprs = Vector{Pair{Symbol, Expr}}() # cache for partial expressions in a :let - # see https://github.com/JuliaLang/julia/blob/bb5b98e72a151c41471d8cc14cacb495d647fb7f/base/views.jl#L17-L75 - inds, _ = foldl(indexing, init=(Expr[], head)) do (inds, partial), ixs + for ixs in indexing S = (partial == head) ? head : gensym(:S) used_S = false @@ -410,13 +416,16 @@ function vinds(expr, head = vsym(expr)) partial = Expr(:ref, partial, ixs...) end - inds = push!(inds, Expr(:tuple, ixs...)) - inds, partial + push!(inds, Expr(:tuple, ixs...)) end tuple_expr = Expr(:tuple, inds...) - cached_assignments = [:($S = $partial) for (S, partial) in cached_exprs] - return Expr(:let, Expr(:block, cached_assignments...), tuple_expr) + if length(cached_exprs) > 0 + cached_assignments = [:($S = $partial) for (S, partial) in cached_exprs] + return Expr(:let, Expr(:block, cached_assignments...), tuple_expr) + else + return tuple_expr + end end _straighten_indexing(expr::Symbol) = Vector{Any}[] From c3678255b84832dd16caeec8a94db45e14be27df Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Mon, 19 Jul 2021 12:50:00 +0200 Subject: [PATCH 05/10] Fix docstrings with eval --- src/varname.jl | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index b2685c20..0ebf52e0 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -361,21 +361,17 @@ suitable for input of the [`VarName`](@ref) constructor. ## Examples ```jldoctest -julia> vinds(:(x[end])) -:((((lastindex)(x),),)) +julia> x = [1]; eval(vinds(:(x[end]))) +((1,),) -julia> vinds(:(x[1, end])) -:(((1, (lastindex)(x, 2)),)) +julia> x = [1 2]; eval(vinds(:(x[1, end]))) +((1, 2),) -julia> vinds(:(x[1][end])) -:(let var"##S#321" = x[1] - ((1,), ((lastindex)(var"##S#321"),)) - end) +julia> x = [[1]]; eval(vinds(:(x[1][end]))) +((1,), (1,)) -julia> vinds(:(x[1][2, end, :][3])) -:(let var"##S#322" = x[1] - ((1,), (2, (lastindex)(var"##S#322", 2), :), (3,)) - end) +julia> x = [fill([1,2,3], 2,2,2)]; eval(vinds(:(x[begin][2, end, :][3]))) +((1,), (2, 2, Colon()), (3,)) ``` """ function vinds(expr, head = vsym(expr)) From 5610351f1f4ae7aa0729344b49870166f9c5ffc9 Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Mon, 19 Jul 2021 17:33:14 +0200 Subject: [PATCH 06/10] Fix tests on versions before `begin` indexing --- src/varname.jl | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 0ebf52e0..3d4e1ccb 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -346,12 +346,15 @@ end _replace_ref_begin_end(ex, withex) = Base.replace_ref_end_!(copy(ex), withex) _replace_ref_begin_end(ex::Symbol, withex) = Base.replace_ref_end_!(ex, withex) _index_replacement_for(s) = :($lastindex($s)) + _index_replacement_for(s, n) = :($lastindex($s, $n)) else _replace_ref_begin_end(ex, withex) = Base.replace_ref_begin_end_!(copy(ex), withex) _replace_ref_begin_end(ex::Symbol, withex) = Base.replace_ref_begin_end_!(ex, withex) _index_replacement_for(s) = :($firstindex($s)), :($lastindex($s)) + _index_replacement_for(s, n) = :($firstindex($s, $n)), :($lastindex($s, $n)) end + """ vinds(expr) @@ -361,17 +364,24 @@ suitable for input of the [`VarName`](@ref) constructor. ## Examples ```jldoctest -julia> x = [1]; eval(vinds(:(x[end]))) -((1,),) +julia> x = [10, 20, 30]; eval(vinds(:(x[end]))) +((3,),) + -julia> x = [1 2]; eval(vinds(:(x[1, end]))) +julia> x = [10 20]; eval(vinds(:(x[1, end]))) ((1, 2),) -julia> x = [[1]]; eval(vinds(:(x[1][end]))) -((1,), (1,)) +julia> x = [[1, 2]]; eval(vinds(:(x[1][end]))) +((1,), (2,)) + +julia> x = [fill([[10], [20, 30]], 2, 2, 2)] + if VERSION < v"1.5.0-DEV.666" + eval(vinds(:(x[1][2, end, :][2][end]))) + else + eval(vinds(Meta.parse("x[begin][2, end, :][2][end]"))) + end +((1,), (2, 2, Colon()), (2,), (2,)) -julia> x = [fill([1,2,3], 2,2,2)]; eval(vinds(:(x[begin][2, end, :][3]))) -((1,), (2, 2, Colon()), (3,)) ``` """ function vinds(expr, head = vsym(expr)) @@ -387,27 +397,18 @@ function vinds(expr, head = vsym(expr)) nixs = length(ixs) if nixs == 1 - ixs[1], used = _replace_ref_begin_end( - ixs[1], - (:($firstindex($S)), :($lastindex($S))) - ) + ixs[1], used = _replace_ref_begin_end(ixs[1], _index_replacement_for(S)) used_S |= used elseif nixs > 1 for i in eachindex(ixs) - ixs[i], used = _replace_ref_begin_end( - ixs[i], - (:($firstindex($S, $i)), :($lastindex($S, $i))) - ) + ixs[i], used = _replace_ref_begin_end(ixs[i], _index_replacement_for(S, i)) used_S |= used end end if used_S && partial !== head push!(cached_exprs, S => partial) - end - - if length(cached_exprs) > 0 && cached_exprs[end][2] == partial - partial = Expr(:ref, cached_exprs[end][1], ixs...) + partial = Expr(:ref, S, ixs...) else partial = Expr(:ref, partial, ixs...) end From 94c6e1581f5005f8ada9be32a3e6a2506569d00e Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Mon, 19 Jul 2021 18:00:27 +0200 Subject: [PATCH 07/10] Add more documentation comments --- src/varname.jl | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 3d4e1ccb..41b47de1 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -359,7 +359,7 @@ end vinds(expr) Return the indexing part of the [`@varname`](@ref)-compatible expression `expr` as an expression -suitable for input of the [`VarName`](@ref) constructor. +suitable for input of the [`VarName`](@ref) constructor (i.e., a tuple of tuples). ## Examples @@ -392,14 +392,17 @@ function vinds(expr, head = vsym(expr)) cached_exprs = Vector{Pair{Symbol, Expr}}() # cache for partial expressions in a :let for ixs in indexing + # S becomes the name of the cached variable S = (partial == head) ? head : gensym(:S) used_S = false nixs = length(ixs) if nixs == 1 + # for 1D indexing, just use `lastindex(x)` ixs[1], used = _replace_ref_begin_end(ixs[1], _index_replacement_for(S)) used_S |= used elseif nixs > 1 + # otherwise, we need `lastindex(x, i)` for i in eachindex(ixs) ixs[i], used = _replace_ref_begin_end(ixs[i], _index_replacement_for(S, i)) used_S |= used @@ -407,6 +410,8 @@ function vinds(expr, head = vsym(expr)) end if used_S && partial !== head + # cache that expression if we actually used it, and use the new name in the + # partial expression push!(cached_exprs, S => partial) partial = Expr(:ref, S, ixs...) else @@ -415,16 +420,34 @@ function vinds(expr, head = vsym(expr)) push!(inds, Expr(:tuple, ixs...)) end - + + # finally make the tuple of tuples tuple_expr = Expr(:tuple, inds...) - if length(cached_exprs) > 0 + + if length(cached_exprs) == 0 + return tuple_expr + else + # construct one big let expression cached_assignments = [:($S = $partial) for (S, partial) in cached_exprs] return Expr(:let, Expr(:block, cached_assignments...), tuple_expr) - else - return tuple_expr end end + +""" + _straighten_indexing(expr) + +Extract a list of lists of (raw) indices of an iterated `:ref` expression. + +```julia +julia> _straighten_indexing(:(x[begin][2, end, :][2][end])) +4-element Array{Array{Any,1},1}: + [:begin] + [2, :end, :(:)] + [2] + [:end] +``` +""" _straighten_indexing(expr::Symbol) = Vector{Any}[] function _straighten_indexing(expr::Expr) if Meta.isexpr(expr, :ref) From 6b659872c88db345d5901754b11c314ba22dd2be Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Sat, 24 Jul 2021 15:12:41 +0200 Subject: [PATCH 08/10] Switch from :ref to maybeview --- src/varname.jl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 41b47de1..9be5a8e3 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -374,6 +374,9 @@ julia> x = [10 20]; eval(vinds(:(x[1, end]))) julia> x = [[1, 2]]; eval(vinds(:(x[1][end]))) ((1,), (2,)) +julia> x = ([1, 2], ); eval(vinds(:(x[1][end]))) # tuple +((1,), (2,)) + julia> x = [fill([[10], [20, 30]], 2, 2, 2)] if VERSION < v"1.5.0-DEV.666" eval(vinds(:(x[1][2, end, :][2][end]))) @@ -389,7 +392,7 @@ function vinds(expr, head = vsym(expr)) indexing = _straighten_indexing(expr) inds = Expr[] # collection of result indices partial = head # partial :ref expressions, used in caching - cached_exprs = Vector{Pair{Symbol, Expr}}() # cache for partial expressions in a :let + cached_exprs = Vector{Pair{Symbol, Expr}}() # cache for partial expressions going into a let for ixs in indexing # S becomes the name of the cached variable @@ -413,9 +416,9 @@ function vinds(expr, head = vsym(expr)) # cache that expression if we actually used it, and use the new name in the # partial expression push!(cached_exprs, S => partial) - partial = Expr(:ref, S, ixs...) + partial = Expr(:call, Base.maybeview, S, ixs...) else - partial = Expr(:ref, partial, ixs...) + partial = Expr(:call, Base.maybeview, partial, ixs...) end push!(inds, Expr(:tuple, ixs...)) From ae461c5af8561452da87960e4ec50f0b7696d91a Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Sat, 24 Jul 2021 15:24:37 +0200 Subject: [PATCH 09/10] Improve docstrings/comments --- src/varname.jl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/varname.jl b/src/varname.jl index 9be5a8e3..f53909c3 100644 --- a/src/varname.jl +++ b/src/varname.jl @@ -224,10 +224,6 @@ A macro that returns an instance of [`VarName`](@ref) given a symbol or indexing The `sym` value is taken from the actual variable name, and the index values are put appropriately into the constructor (and resolved at runtime). -NB: `begin` and `end` indexing can be used, but remember that they depend on _runtime values_ -- -their usage requires that the array over which the indexing expression is defined is defined, in -order for `firstindex` and `lastindex` to work in the expanded code. - ## Examples ```jldoctest @@ -250,6 +246,13 @@ julia> let a = [42]; @varname(a[1][end][3]); end a[1][1][3] ``` +!!! warning + As you can see in the last example, `@varname` does not do any bounds checking! + + Only `begin` and `end` indexing, which depend on the _runtime size_ of the array, does that! + Their usage _requires_ that the array over which the indexing expression is defined, in + order for `firstindex` and `lastindex` to work in the expanded code. + !!! compat "Julia 1.5" Using `begin` in an indexing expression to refer to the first index requires at least Julia 1.5. @@ -401,11 +404,11 @@ function vinds(expr, head = vsym(expr)) nixs = length(ixs) if nixs == 1 - # for 1D indexing, just use `lastindex(x)` + # for linear indexing, we use `lastindex(x)` ixs[1], used = _replace_ref_begin_end(ixs[1], _index_replacement_for(S)) used_S |= used elseif nixs > 1 - # otherwise, we need `lastindex(x, i)` + # for cartesian indexing, we need `lastindex(x, i)` for i in eachindex(ixs) ixs[i], used = _replace_ref_begin_end(ixs[i], _index_replacement_for(S, i)) used_S |= used From 41ab5ea30fea6a979d166bc9098ced7086c425ac Mon Sep 17 00:00:00 2001 From: Philipp Gabler Date: Sat, 24 Jul 2021 15:25:05 +0200 Subject: [PATCH 10/10] Bump version --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index ed5ef88a..bfe867a7 100644 --- a/Project.toml +++ b/Project.toml @@ -3,7 +3,7 @@ uuid = "7a57a42e-76ec-4ea3-a279-07e840d6d9cf" keywords = ["probablistic programming"] license = "MIT" desc = "Common interfaces for probabilistic programming" -version = "0.2.0" +version = "0.2.1" [deps] AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"