Skip to content

Commit a848290

Browse files
committed
Merge branch 'mhauru/vnt-vntsize' into mhauru/vnt-for-fastldf
2 parents bb22bc3 + cacd426 commit a848290

File tree

5 files changed

+29
-12
lines changed

5 files changed

+29
-12
lines changed

docs/src/api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ SimpleVarInfo
377377

378378
```@docs
379379
DynamicPPL.VarNamedTuples.VarNamedTuple
380+
DynamicPPL.VarNamedTuples.vnt_size
380381
```
381382

382383
### Accumulators

docs/src/internals/varnamedtuple.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ For instance, if `setindex!!(vnt, @varname(a[1:5]), val)` has been set, then the
168168
Not `@varname(a[1:10])`, nor `@varname(a[3])`, nor for anything else that overlaps with `@varname(a[1:5])`.
169169
`haskey` likewise only returns true for `@varname(a[1:5])`, and `keys(vnt)` only has that as an element.
170170

171+
The size of a value, for the purposes of inserting it into a `PartialArray`, is determined by a call to `vnt_size`.
172+
`vnt_size` falls back to calling `Base.size`.
173+
The reason we define a distinct function is to be able to control its behaviour, if necessary, without type piracy.
174+
171175
## Limitations
172176

173177
This design has a several of benefits, for performance and generality, but it also has limitations:

src/DynamicPPL.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ abstract type AbstractVarInfo <: AbstractModelTrace end
185185
# Necessary forward declarations
186186
include("utils.jl")
187187
include("varnamedtuple.jl")
188-
using .VarNamedTuples: VarNamedTuple
188+
using .VarNamedTuples: VarNamedTuples, VarNamedTuple
189189
include("contexts.jl")
190190
include("contexts/default.jl")
191191
include("contexts/init.jl")

src/contexts/init.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ struct RangeAndLinked{T<:Tuple}
215215
original_size::T
216216
end
217217

218-
Base.size(ral::RangeAndLinked) = ral.original_size
218+
VarNamedTuples.vnt_size(ral::RangeAndLinked) = ral.original_size
219219

220220
"""
221221
VectorWithRanges{Tlink}(

src/varnamedtuple.jl

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ using BangBang
88
using Accessors
99
using ..DynamicPPL: _compose_no_identity
1010

11-
export VarNamedTuple
11+
export VarNamedTuple, vnt_size
1212

1313
# We define our own getindex, setindex!!, and haskey functions, which we use to
1414
# get/set/check values in VarNamedTuple and PartialArray. We do this because we want to be
@@ -81,6 +81,17 @@ const INDEX_TYPES = Union{Integer,AbstractUnitRange,Colon,AbstractPPL.Concretize
8181
_unwrap_concretized_slice(cs::AbstractPPL.ConcretizedSlice) = cs.range
8282
_unwrap_concretized_slice(x::Union{Integer,AbstractUnitRange,Colon}) = x
8383

84+
"""
85+
vnt_size(x)
86+
87+
Get the size of an object `x` for use in `VarNamedTuple` and `PartialArray`.
88+
89+
By default, this falls back onto `Base.size`, but can be overloaded for custom types.
90+
This notion of type is used to determine whether a value can be set into a `PartialArray`
91+
as a block, see the docstring of `PartialArray` and `ArrayLikeBlock` for details.
92+
"""
93+
vnt_size(x) = size(x)
94+
8495
"""
8596
ArrayLikeBlock{T,I}
8697
@@ -156,11 +167,12 @@ Like `Base.Array`s, `PartialArray`s have a well-defined, compile-time-known elem
156167
157168
One can set values in a `PartialArray` either element-by-element, or with ranges like
158169
`arr[1:3,2] = [5,10,15]`. When setting values over a range of indices, the value being set
159-
must either be an `AbstractArray` or otherwise something for which `size(value)` is defined,
160-
and the size mathces the range. If the value is an `AbstractArray`, the elements are copied
161-
individually, but if it is not, the value is stored as a block, that takes up the whole
162-
range, e.g. `[1:3,2]`, but is only a single object. Getting such a block-value must be done
163-
with the exact same range of indices, otherwise an error is thrown.
170+
must either be an `AbstractArray` or otherwise something for which `vnt_size(value)` or
171+
`Base.size(value)` (which `vnt_size` falls back onto) is defined, and the size matches the
172+
range. If the value is an `AbstractArray`, the elements are copied individually, but if it
173+
is not, the value is stored as a block, that takes up the whole range, e.g. `[1:3,2]`, but
174+
is only a single object. Getting such a block-value must be done with the exact same range
175+
of indices, otherwise an error is thrown.
164176
165177
If the element type of a `PartialArray` is not concrete, any call to `setindex!!` will check
166178
if, after the new value has been set, the element type can be made more concrete. If so,
@@ -594,7 +606,7 @@ The value only depends on the types of the arguments, and should be constant pro
594606
function _needs_arraylikeblock(value, inds::Vararg{INDEX_TYPES})
595607
return _is_multiindex(inds) &&
596608
!isa(value, AbstractArray) &&
597-
hasmethod(size, Tuple{typeof(value)})
609+
hasmethod(vnt_size, Tuple{typeof(value)})
598610
end
599611

600612
function _setindex!!(pa::PartialArray, value, inds::Vararg{INDEX_TYPES})
@@ -610,11 +622,11 @@ function _setindex!!(pa::PartialArray, value, inds::Vararg{INDEX_TYPES})
610622
new_data = pa.data
611623
if _needs_arraylikeblock(value, inds...)
612624
inds_size = reduce((x, y) -> tuple(x..., y...), map(size, inds))
613-
if size(value) != inds_size
625+
if vnt_size(value) != inds_size
614626
throw(
615627
DimensionMismatch(
616-
"Assigned value has size $(size(value)), which does not match the " *
617-
"size implied by the indices $(map(x -> _length_needed(x), inds)).",
628+
"Assigned value has size $(vnt_size(value)), which does not match " *
629+
"the size implied by the indices $(map(x -> _length_needed(x), inds)).",
618630
),
619631
)
620632
end

0 commit comments

Comments
 (0)