From f6dd7ecc9c1abb92cdc3be2ee8d3489232719ef6 Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Fri, 16 Feb 2024 13:58:54 +0000 Subject: [PATCH 1/9] remove `BangBang.possible` --- src/utils.jl | 37 ---------------- test/utils.jl | 115 -------------------------------------------------- 2 files changed, 152 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index b447fed53..d515a303d 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -539,43 +539,6 @@ function remove_parent_lens(vn_parent::VarName{sym}, vn_child::VarName{sym}) whe 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. """ diff --git a/test/utils.jl b/test/utils.jl index a2d6f46fb..1fcf09ef1 100644 --- a/test/utils.jl +++ b/test/utils.jl @@ -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 From 58dec49225749f1f35ef417b9a36f88f5300e34b Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Fri, 16 Feb 2024 15:26:30 +0000 Subject: [PATCH 2/9] version bumps --- Project.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Project.toml b/Project.toml index 6510e7ea0..9b7ce5cb4 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "DynamicPPL" uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8" -version = "0.24.7" +version = "0.24.8" [deps] ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b" @@ -28,7 +28,7 @@ ZygoteRules = "700de1a5-db45-46bc-99cf-38207098b444" ADTypes = "0.2" AbstractMCMC = "5" AbstractPPL = "0.7" -BangBang = "0.3" +BangBang = "0.4.1" Bijectors = "0.13" ChainRulesCore = "1" Compat = "4" From d3cd5e873723d35d69ea4b0b69ec7a0482b647b2 Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Thu, 29 Feb 2024 08:05:41 +0000 Subject: [PATCH 3/9] remove dep `MLUtils` --- docs/Project.toml | 2 -- docs/src/tutorials/prob-interface.md | 20 ++++++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/Project.toml b/docs/Project.toml index 48ebe173c..271976d0e 100644 --- a/docs/Project.toml +++ b/docs/Project.toml @@ -5,7 +5,6 @@ Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4" FillArrays = "1a297f60-69ca-5386-bcde-b61e274b549b" LogDensityProblems = "6fdf6af0-433a-55f7-b3ed-c6c6e0b8df7c" MCMCChains = "c7f686f2-ff18-58e9-bc7b-31028e88f75d" -MLUtils = "f1d291b0-491e-4a28-83b9-f70985020b54" Setfield = "efcf1570-3423-57d1-acb7-fd33fddbac46" StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3" @@ -16,6 +15,5 @@ Documenter = "1" FillArrays = "0.13, 1" LogDensityProblems = "2" MCMCChains = "5, 6" -MLUtils = "0.3, 0.4" Setfield = "0.7.1, 0.8, 1" StableRNGs = "1" diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index dc9f50204..cccebdaf3 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -107,12 +107,11 @@ To give an example of the probability interface in use, we can use it to estimat In cross-validation, we split the dataset into several equal parts. Then, we choose one of these sets to serve as the validation set. Here, we measure fit using the cross entropy (Bayes loss).[^1] +(For the sake of simplicity, in the following code, we enforce that `nfolds` ) ```@example probinterface -using MLUtils - function cross_val( - dataset::AbstractVector{<:Real}; + dataset::Vector{<:Real}; nfolds::Int=5, nsamples::Int=1_000, rng::Random.AbstractRNG=Random.default_rng(), @@ -121,7 +120,20 @@ function cross_val( model = gdemo(1) | (x=[first(dataset)],) loss = zero(logjoint(model, rand(rng, model))) - for (train, validation) in kfolds(dataset, nfolds) + # prepare the K-folds + fold_size = div(length(dataset), nfolds) + if length(dataset) % nfolds != 0 + error("The number of folds must divide the number of data points.") + end + splits = Vector{Tuple{SubArray, SubArray}}(undef, nfolds) + + for i in 1:nfolds + start_idx, end_idx = (i-1)*fold_size + 1, i*fold_size + train_set_indices = [1:start_idx-1; end_idx+1:length(dataset)] + splits[i] = (view(dataset, train_set_indices), view(dataset, start_idx:end_idx)) + end + + for (train, validation) in splits # First, we train the model on the training set, i.e., we obtain samples from the posterior. # For normally-distributed data, the posterior can be computed in closed form. # For general models, however, typically samples will be generated using MCMC with Turing. From 4e4a994e9dab5b0877df71ae044c81387de6dd7b Mon Sep 17 00:00:00 2001 From: Xianda Sun <5433119+sunxd3@users.noreply.github.com> Date: Thu, 29 Feb 2024 08:08:25 +0000 Subject: [PATCH 4/9] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- docs/src/tutorials/prob-interface.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index cccebdaf3..7ff81cf25 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -125,11 +125,11 @@ function cross_val( if length(dataset) % nfolds != 0 error("The number of folds must divide the number of data points.") end - splits = Vector{Tuple{SubArray, SubArray}}(undef, nfolds) - + splits = Vector{Tuple{SubArray,SubArray}}(undef, nfolds) + for i in 1:nfolds - start_idx, end_idx = (i-1)*fold_size + 1, i*fold_size - train_set_indices = [1:start_idx-1; end_idx+1:length(dataset)] + start_idx, end_idx = (i - 1) * fold_size + 1, i * fold_size + train_set_indices = [1:(start_idx - 1); (end_idx + 1):length(dataset)] splits[i] = (view(dataset, train_set_indices), view(dataset, start_idx:end_idx)) end From ef668a33d2aa562d2c51fa1b1eefa2164b1516d6 Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Thu, 29 Feb 2024 12:57:46 +0000 Subject: [PATCH 5/9] finish sentence --- docs/src/tutorials/prob-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index 7ff81cf25..5a16b0db1 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -107,7 +107,7 @@ To give an example of the probability interface in use, we can use it to estimat In cross-validation, we split the dataset into several equal parts. Then, we choose one of these sets to serve as the validation set. Here, we measure fit using the cross entropy (Bayes loss).[^1] -(For the sake of simplicity, in the following code, we enforce that `nfolds` ) +(For the sake of simplicity, in the following code, we enforce that `nfolds` must divide the number of data points. For a more competent implementation, see [MLUtils.jl](https://juliaml.github.io/MLUtils.jl/dev/api/#MLUtils.kfolds).) ```@example probinterface function cross_val( From c901f820bcf469990df5fc294ab47d793ba8cdda Mon Sep 17 00:00:00 2001 From: Xianda Sun <5433119+sunxd3@users.noreply.github.com> Date: Thu, 29 Feb 2024 12:58:26 +0000 Subject: [PATCH 6/9] Update docs/src/tutorials/prob-interface.md Co-authored-by: David Widmann --- docs/src/tutorials/prob-interface.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index 5a16b0db1..cc19bf050 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -121,8 +121,8 @@ function cross_val( loss = zero(logjoint(model, rand(rng, model))) # prepare the K-folds - fold_size = div(length(dataset), nfolds) - if length(dataset) % nfolds != 0 + fold_size, remaining = divrem(length(dataset), nfolds) + if remaining != 0 error("The number of folds must divide the number of data points.") end splits = Vector{Tuple{SubArray,SubArray}}(undef, nfolds) From f20dafe95146b5f3dad9589b81fbb01577ddaa97 Mon Sep 17 00:00:00 2001 From: Xianda Sun <5433119+sunxd3@users.noreply.github.com> Date: Thu, 29 Feb 2024 12:59:11 +0000 Subject: [PATCH 7/9] Update docs/src/tutorials/prob-interface.md Co-authored-by: David Widmann --- docs/src/tutorials/prob-interface.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index cc19bf050..2da5c3201 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -125,12 +125,13 @@ function cross_val( if remaining != 0 error("The number of folds must divide the number of data points.") end - splits = Vector{Tuple{SubArray,SubArray}}(undef, nfolds) - - for i in 1:nfolds - start_idx, end_idx = (i - 1) * fold_size + 1, i * fold_size - train_set_indices = [1:(start_idx - 1); (end_idx + 1):length(dataset)] - splits[i] = (view(dataset, train_set_indices), view(dataset, start_idx:end_idx)) + first_idx = firstindex(dataset) + last_idx = lastindex(dataset) + splits = map(0:(nfolds - 1)) do i + start_idx = first_idx + i * fold_size + end_idx = start_idx + fold_size + train_set_indices = [first_idx:start_idx; (end_idx + 1):last_idx] + return (view(dataset, train_set_indices), view(dataset, (start_idx + 1):end_idx)) end for (train, validation) in splits From 9934f99d2c7503d5843e71e893dd36de1bf2ffc2 Mon Sep 17 00:00:00 2001 From: Xianda Sun Date: Thu, 29 Feb 2024 13:12:03 +0000 Subject: [PATCH 8/9] make `kfolds` a function --- docs/src/tutorials/prob-interface.md | 31 +++++++++++++++------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index 2da5c3201..683396550 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -110,17 +110,8 @@ Here, we measure fit using the cross entropy (Bayes loss).[^1] (For the sake of simplicity, in the following code, we enforce that `nfolds` must divide the number of data points. For a more competent implementation, see [MLUtils.jl](https://juliaml.github.io/MLUtils.jl/dev/api/#MLUtils.kfolds).) ```@example probinterface -function cross_val( - dataset::Vector{<:Real}; - nfolds::Int=5, - nsamples::Int=1_000, - rng::Random.AbstractRNG=Random.default_rng(), -) - # Initialize `loss` in a way such that the loop below does not change its type - model = gdemo(1) | (x=[first(dataset)],) - loss = zero(logjoint(model, rand(rng, model))) - - # prepare the K-folds +# Calculate the train/validation splits across `nfolds` partitions, assume `length(dataset)` divides `nfolds` +function kfolds(dataset::Array{<:Real}, nfolds::Int) fold_size, remaining = divrem(length(dataset), nfolds) if remaining != 0 error("The number of folds must divide the number of data points.") @@ -130,11 +121,23 @@ function cross_val( splits = map(0:(nfolds - 1)) do i start_idx = first_idx + i * fold_size end_idx = start_idx + fold_size - train_set_indices = [first_idx:start_idx; (end_idx + 1):last_idx] - return (view(dataset, train_set_indices), view(dataset, (start_idx + 1):end_idx)) + train_set_indices = [first_idx:start_idx-1; end_idx:last_idx] + return (view(dataset, train_set_indices), view(dataset, start_idx:end_idx-1)) end + return splits +end + +function cross_val( + dataset::Vector{<:Real}; + nfolds::Int=5, + nsamples::Int=1_000, + rng::Random.AbstractRNG=Random.default_rng(), +) + # Initialize `loss` in a way such that the loop below does not change its type + model = gdemo(1) | (x=[first(dataset)],) + loss = zero(logjoint(model, rand(rng, model))) - for (train, validation) in splits + for (train, validation) in kfolds(dataset, nfolds) # First, we train the model on the training set, i.e., we obtain samples from the posterior. # For normally-distributed data, the posterior can be computed in closed form. # For general models, however, typically samples will be generated using MCMC with Turing. From 00a3f0b015ed0fc2391dc9ebd9ae8a1217a908e0 Mon Sep 17 00:00:00 2001 From: Xianda Sun <5433119+sunxd3@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:18:47 +0000 Subject: [PATCH 9/9] Update docs/src/tutorials/prob-interface.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- docs/src/tutorials/prob-interface.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/tutorials/prob-interface.md b/docs/src/tutorials/prob-interface.md index 683396550..330fa931a 100644 --- a/docs/src/tutorials/prob-interface.md +++ b/docs/src/tutorials/prob-interface.md @@ -121,8 +121,8 @@ function kfolds(dataset::Array{<:Real}, nfolds::Int) splits = map(0:(nfolds - 1)) do i start_idx = first_idx + i * fold_size end_idx = start_idx + fold_size - train_set_indices = [first_idx:start_idx-1; end_idx:last_idx] - return (view(dataset, train_set_indices), view(dataset, start_idx:end_idx-1)) + train_set_indices = [first_idx:(start_idx - 1); end_idx:last_idx] + return (view(dataset, train_set_indices), view(dataset, start_idx:(end_idx - 1))) end return splits end