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.

Sign up for GitHub

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

Remove BangBang.possible #579

Merged
merged 11 commits into from
Apr 18, 2024
Merged

Remove BangBang.possible #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f6dd7ec
remove `BangBang.possible`
sunxd3 Feb 16, 2024
58dec49
version bumps
sunxd3 Feb 16, 2024
d3cd5e8
remove dep `MLUtils`
sunxd3 Feb 29, 2024
4e4a994
Apply suggestions from code review
sunxd3 Feb 29, 2024
ef668a3
finish sentence
sunxd3 Feb 29, 2024
c901f82
Update docs/src/tutorials/prob-interface.md
sunxd3 Feb 29, 2024
f20dafe
Update docs/src/tutorials/prob-interface.md
sunxd3 Feb 29, 2024
9934f99
make `kfolds` a function
sunxd3 Feb 29, 2024
00a3f0b
Update docs/src/tutorials/prob-interface.md
sunxd3 Feb 29, 2024
bfd83dd
Merge branch 'master' into sunxd/remove_bangbang.hack
sunxd3 Mar 1, 2024
e749bca
Merge branch 'master' into sunxd/remove_bangbang.hack
yebai Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ADTypes = "0.2"
AbstractMCMC = "5"
AbstractPPL = "0.8.4"
Accessors = "0.1"
BangBang = "0.4"
BangBang = "0.4.1"
Bijectors = "0.13.9"
ChainRulesCore = "1"
Compat = "4"
Expand Down
37 changes: 0 additions & 37 deletions src/utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,43 +548,6 @@ function remove_parent_optic(vn_parent::VarName{sym}, vn_child::VarName{sym}) wh
return child
end

# HACK: All of these are related to https://github.com/JuliaFolds/BangBang.jl/issues/233
# and https://github.com/JuliaFolds/BangBang.jl/pull/238, https://github.com/JuliaFolds2/BangBang.jl/pull/16.
# This avoids type-instability in `dot_assume` for `SimpleVarInfo`.
# The following code a copy from https://github.com/JuliaFolds2/BangBang.jl/pull/16 authored by torfjelde
# Default implementation for `_setindex!` with `AbstractArray`.
# But this will return `false` even in cases such as
#
# setindex!!([1, 2, 3], [4, 5, 6], :)
#
# because `promote_type(eltype(C), T) <: eltype(C)` is `false`.
# To address this, we specialize on the case where `T<:AbstractArray`.
# In addition, we need to support a wide range of indexing behaviors:
#
# We also need to ensure that the dimensionality of the index is
# valid, i.e. that we're not returning `true` in cases such as
#
# setindex!!([1, 2, 3], [4, 5], 1)
#
# which should return `false`.
_index_dimension(::Any) = 0
_index_dimension(::Colon) = 1
_index_dimension(::AbstractVector) = 1
_index_dimension(indices::Tuple) = sum(map(_index_dimension, indices))

function BangBang.possible(
::typeof(BangBang._setindex!), ::C, ::T, indices::Vararg
) where {M,C<:AbstractArray{<:Real},T<:AbstractArray{<:Real,M}}
return BangBang.implements(setindex!, C) &&
promote_type(eltype(C), eltype(T)) <: eltype(C) &&
# This will still return `false` for scenarios such as
#
# setindex!!([1, 2, 3], [4, 5, 6], :, 1)
#
# which are in fact valid. However, this cases are rare.
(_index_dimension(indices) == M || _index_dimension(indices) == 1)
end

# HACK(torfjelde): This makes it so it works on iterators, etc. by default.
# TODO(torfjelde): Do better.
"""
Expand Down
115 changes: 0 additions & 115 deletions test/utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,119 +48,4 @@
x = rand(dist)
@test vectorize(dist, x) == vec(x.UL)
end

@testset "BangBang.possible" begin
using DynamicPPL.BangBang: setindex!!

# Some utility methods for testing `setindex!`.
test_linear_index_only(::Tuple, ::AbstractArray) = false
test_linear_index_only(inds::NTuple{1}, ::AbstractArray) = true
test_linear_index_only(inds::NTuple{1}, ::AbstractVector) = false

function replace_colon_with_axis(inds::Tuple, x)
ntuple(length(inds)) do i
inds[i] isa Colon ? axes(x, i) : inds[i]
end
end
function replace_colon_with_vector(inds::Tuple, x)
ntuple(length(inds)) do i
inds[i] isa Colon ? collect(axes(x, i)) : inds[i]
end
end
function replace_colon_with_range(inds::Tuple, x)
ntuple(length(inds)) do i
inds[i] isa Colon ? (1:size(x, i)) : inds[i]
end
end
function replace_colon_with_booleans(inds::Tuple, x)
ntuple(length(inds)) do i
inds[i] isa Colon ? trues(size(x, i)) : inds[i]
end
end

function replace_colon_with_range_linear(inds::NTuple{1}, x::AbstractArray)
return inds[1] isa Colon ? (1:length(x),) : inds
end

@testset begin
@test setindex!!((1, 2, 3), :two, 2) === (1, :two, 3)
@test setindex!!((a=1, b=2, c=3), :two, :b) === (a=1, b=:two, c=3)
@test setindex!!([1, 2, 3], :two, 2) == [1, :two, 3]
@test setindex!!(Dict{Symbol,Int}(:a => 1, :b => 2), 10, :a) ==
Dict(:a => 10, :b => 2)
@test setindex!!(Dict{Symbol,Int}(:a => 1, :b => 2), 3, "c") ==
Dict(:a => 1, :b => 2, "c" => 3)
end

@testset "mutation" begin
@testset "without type expansion" begin
for args in [([1, 2, 3], 20, 2), (Dict(:a => 1, :b => 2), 10, :a)]
@test setindex!!(args...) === args[1]
end
end

@testset "with type expansion" begin
@test setindex!!([1, 2, 3], [4, 5], 1) == [[4, 5], 2, 3]
@test setindex!!([1, 2, 3], [4, 5, 6], :, 1) == [4, 5, 6]
end
end

@testset "slices" begin
@testset "$(typeof(x)) with $(src_idx)" for (x, src_idx) in [
# Vector.
(randn(2), (:,)),
(randn(2), (1:2,)),
# Matrix.
(randn(2, 3), (:,)),
(randn(2, 3), (:, 1)),
(randn(2, 3), (:, 1:3)),
# 3D array.
(randn(2, 3, 4), (:, 1, :)),
(randn(2, 3, 4), (:, 1:3, :)),
(randn(2, 3, 4), (1, 1:3, :)),
]
# Base case.
@test @inferred(setindex!!(x, x[src_idx...], src_idx...)) === x

# If we have `Colon` in the index, we replace this with other equivalent indices.
if any(Base.Fix2(isa, Colon), src_idx)
if test_linear_index_only(src_idx, x)
# With range instead of `Colon`.
@test @inferred(
setindex!!(
x,
x[src_idx...],
replace_colon_with_range_linear(src_idx, x)...,
)
) === x
else
# With axis instead of `Colon`.
@test @inferred(
setindex!!(
x, x[src_idx...], replace_colon_with_axis(src_idx, x)...
)
) === x
# With range instead of `Colon`.
@test @inferred(
setindex!!(
x, x[src_idx...], replace_colon_with_range(src_idx, x)...
)
) === x
# With vectors instead of `Colon`.
@test @inferred(
setindex!!(
x, x[src_idx...], replace_colon_with_vector(src_idx, x)...
)
) === x
# With boolean index instead of `Colon`.
@test @inferred(
setindex!!(
x, x[src_idx...], replace_colon_with_booleans(src_idx, x)...
)
) === x
end
end
end
end
end
end
Loading