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.

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

Fix method ambiguities & unbound parameters + add Aqua tests #1804

Merged
merged 3 commits into from
Dec 8, 2023
Merged
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -30,15 +30,26 @@ DistributionsDensityInterfaceExt = "DensityInterface"
DistributionsTestExt = "Test"

[compat]
Aqua = "0.8"
Calculus = "0.5"
ChainRulesCore = "1"
ChainRulesTestUtils = "1"
DensityInterface = "0.4"
Distributed = "<0.0.1, 1"
FillArrays = "0.9, 0.10, 0.11, 0.12, 0.13, 1"
FiniteDifferences = "0.12"
ForwardDiff = "0.10"
JSON = "0.21"
LinearAlgebra = "<0.0.1, 1"
OffsetArrays = "1"
PDMats = "0.10, 0.11"
Printf = "<0.0.1, 1"
QuadGK = "2"
Random = "<0.0.1, 1"
SparseArrays = "<0.0.1, 1"
SpecialFunctions = "1.2, 2"
StableRNGs = "1"
StaticArrays = "1"
Statistics = "1"
StatsAPI = "1.6"
StatsBase = "0.32, 0.33, 0.34"
@@ -47,6 +58,7 @@ Test = "<0.0.1, 1"
julia = "1.3"

[extras]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
Calculus = "49dc2e85-a5d0-5ad3-a950-438e2897f1b9"
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a"
@@ -62,4 +74,4 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["StableRNGs", "Calculus", "ChainRulesCore", "ChainRulesTestUtils", "DensityInterface", "Distributed", "FiniteDifferences", "ForwardDiff", "JSON", "SparseArrays", "StaticArrays", "Test", "OffsetArrays"]
test = ["Aqua", "StableRNGs", "Calculus", "ChainRulesCore", "ChainRulesTestUtils", "DensityInterface", "Distributed", "FiniteDifferences", "ForwardDiff", "JSON", "SparseArrays", "StaticArrays", "Test", "OffsetArrays"]
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ Distributions.jl
[![Build Status](https://github.com/JuliaStats/Distributions.jl/workflows/CI/badge.svg)](https://github.com/JuliaStats/Distributions.jl/actions)
[![](https://zenodo.org/badge/DOI/10.5281/zenodo.2647458.svg)](https://zenodo.org/record/2647458)
[![Coverage Status](https://coveralls.io/repos/JuliaStats/Distributions.jl/badge.svg?branch=master)](https://coveralls.io/r/JuliaStats/Distributions.jl?branch=master)
[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)

[![](https://img.shields.io/badge/docs-latest-blue.svg)](https://JuliaStats.github.io/Distributions.jl/latest/)
[![](https://img.shields.io/badge/docs-stable-blue.svg)](https://JuliaStats.github.io/Distributions.jl/stable/)
1 change: 1 addition & 0 deletions src/censored.jl
Original file line number Diff line number Diff line change
@@ -431,6 +431,7 @@ _in_open_interval(x::Real, l::Real, ::Nothing) = x > l
_clamp(x, l, u) = clamp(x, l, u)
_clamp(x, ::Nothing, u) = min(x, u)
_clamp(x, l, ::Nothing) = max(x, l)
_clamp(x, ::Nothing, u::Nothing) = x

_to_truncated(d::Censored) = truncated(d.uncensored, d.lower, d.upper)

107 changes: 51 additions & 56 deletions src/common.jl
Original file line number Diff line number Diff line change
@@ -212,13 +212,25 @@ usually it is sufficient to implement `logpdf`.
See also: [`logpdf`](@ref).
"""
@inline function pdf(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}
) where {N}
@boundscheck begin
size(x) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M}
) where {N,M}
if M == N
@boundscheck begin
size(x) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return _pdf(d, x)
else
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return @inbounds map(Base.Fix1(pdf, d), eachvariate(x, variate_form(typeof(d))))
end
return _pdf(d, x)
end

function _pdf(d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}) where {N}
@@ -241,13 +253,25 @@ size of `x`.
See also: [`pdf`](@ref).
"""
@inline function logpdf(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N}
) where {N}
@boundscheck begin
size(x) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M}
) where {N,M}
if M == N
@boundscheck begin
size(x) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return _logpdf(d, x)
else
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return @inbounds map(Base.Fix1(logpdf, d), eachvariate(x, variate_form(typeof(d))))
end
return _logpdf(d, x)
end

# `_logpdf` should be implemented and has no default definition
@@ -272,20 +296,6 @@ Base.@propagate_inbounds function pdf(
return map(Base.Fix1(pdf, d), x)
end

@inline function pdf(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
) where {N,M}
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return @inbounds map(Base.Fix1(pdf, d), eachvariate(x, variate_form(typeof(d))))
end

"""
logpdf(d::Distribution{ArrayLikeVariate{N}}, x) where {N}

@@ -305,20 +315,6 @@ Base.@propagate_inbounds function logpdf(
return map(Base.Fix1(logpdf, d), x)
end

@inline function logpdf(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
) where {N,M}
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return @inbounds map(Base.Fix1(logpdf, d), eachvariate(x, variate_form(typeof(d))))
end

"""
pdf!(out, d::Distribution{ArrayLikeVariate{N}}, x) where {N}

@@ -365,7 +361,7 @@ end
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
"number of dimensions of the variates ($M) must be greater than the dimension of the distribution ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
@@ -414,7 +410,7 @@ See also: [`pdf!`](@ref).
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
"number of dimensions of the variates ($M) must be greater than the dimension of the distribution ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
@@ -445,23 +441,22 @@ be
- an array of dimension `N + 1` with `size(x)[1:N] == size(d)`, or
- an array of arrays `xi` of dimension `N` with `size(xi) == size(d)`.
"""
Base.@propagate_inbounds function loglikelihood(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,N},
) where {N}
return logpdf(d, x)
end
@inline function loglikelihood(
Base.@propagate_inbounds @inline function loglikelihood(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:Real,M},
) where {N,M}
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of `x` ($M) must be greater than number of dimensions of `d` ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
if M == N
return logpdf(d, x)
else
@boundscheck begin
M > N ||
throw(DimensionMismatch(
"number of dimensions of the variates ($M) must be greater than or equal to the dimension of the distribution ($N)"
))
ntuple(i -> size(x, i), Val(N)) == size(d) ||
throw(DimensionMismatch("inconsistent array dimensions"))
end
return @inbounds sum(Base.Fix1(logpdf, d), eachvariate(x, ArrayLikeVariate{N}))
end
return @inbounds sum(Base.Fix1(logpdf, d), eachvariate(x, ArrayLikeVariate{N}))
end
Base.@propagate_inbounds function loglikelihood(
d::Distribution{ArrayLikeVariate{N}}, x::AbstractArray{<:AbstractArray{<:Real,N}},
6 changes: 3 additions & 3 deletions src/deprecates.jl
Original file line number Diff line number Diff line change
@@ -34,9 +34,9 @@ for fun in [:pdf, :logpdf,
fun! = Symbol(fun, '!')

@eval begin
@deprecate ($_fun!)(r::AbstractArray, d::UnivariateDistribution, X::AbstractArray) r .= ($fun).(d, X) false
@deprecate ($fun!)(r::AbstractArray, d::UnivariateDistribution, X::AbstractArray) r .= ($fun).(d, X) false
@deprecate ($fun)(d::UnivariateDistribution, X::AbstractArray) ($fun).(d, X)
@deprecate ($_fun!)(r::AbstractArray{<:Real}, d::UnivariateDistribution, X::AbstractArray{<:Real}) r .= ($fun).(d, X) false
@deprecate ($fun!)(r::AbstractArray{<:Real}, d::UnivariateDistribution, X::AbstractArray{<:Real}) r .= ($fun).(d, X) false
@deprecate ($fun)(d::UnivariateDistribution, X::AbstractArray{<:Real}) ($fun).(d, X)
end
end

16 changes: 8 additions & 8 deletions src/mixtures/mixturemodel.jl
Original file line number Diff line number Diff line change
@@ -362,14 +362,14 @@ end
pdf(d::UnivariateMixture, x::Real) = _mixpdf1(d, x)
logpdf(d::UnivariateMixture, x::Real) = _mixlogpdf1(d, x)

_pdf!(r::AbstractArray, d::UnivariateMixture{Discrete}, x::UnitRange) = _mixpdf!(r, d, x)
_pdf!(r::AbstractArray, d::UnivariateMixture, x::AbstractArray) = _mixpdf!(r, d, x)
_logpdf!(r::AbstractArray, d::UnivariateMixture, x::AbstractArray) = _mixlogpdf!(r, d, x)

_pdf(d::MultivariateMixture, x::AbstractVector) = _mixpdf1(d, x)
_logpdf(d::MultivariateMixture, x::AbstractVector) = _mixlogpdf1(d, x)
_pdf!(r::AbstractArray, d::MultivariateMixture, x::AbstractMatrix) = _mixpdf!(r, d, x)
_logpdf!(r::AbstractArray, d::MultivariateMixture, x::AbstractMatrix) = _mixlogpdf!(r, d, x)
_pdf!(r::AbstractArray{<:Real}, d::UnivariateMixture{Discrete}, x::UnitRange) = _mixpdf!(r, d, x)
_pdf!(r::AbstractArray{<:Real}, d::UnivariateMixture, x::AbstractArray{<:Real}) = _mixpdf!(r, d, x)
_logpdf!(r::AbstractArray{<:Real}, d::UnivariateMixture, x::AbstractArray{<:Real}) = _mixlogpdf!(r, d, x)

_pdf(d::MultivariateMixture, x::AbstractVector{<:Real}) = _mixpdf1(d, x)
_logpdf(d::MultivariateMixture, x::AbstractVector{<:Real}) = _mixlogpdf1(d, x)
_pdf!(r::AbstractArray{<:Real}, d::MultivariateMixture, x::AbstractMatrix{<:Real}) = _mixpdf!(r, d, x)
_logpdf!(r::AbstractArray{<:Real}, d::MultivariateMixture, x::AbstractMatrix{<:Real}) = _mixlogpdf!(r, d, x)


## component-wise pdf and logpdf
2 changes: 1 addition & 1 deletion src/multivariate/mvtdist.jl
Original file line number Diff line number Diff line change
@@ -139,7 +139,7 @@ function _logpdf(d::AbstractMvTDist, x::AbstractVector{T}) where T<:Real
v - shdfhdim * log1p(sqmahal(d, x) / d.df)
end

function _logpdf!(r::AbstractArray, d::AbstractMvTDist, x::AbstractMatrix)
function _logpdf!(r::AbstractArray{<:Real}, d::AbstractMvTDist, x::AbstractMatrix{<:Real})
sqmahal!(r, d, x)
shdfhdim, v = mvtdist_consts(d)
for i = 1:size(x, 2)
14 changes: 8 additions & 6 deletions src/product.jl
Original file line number Diff line number Diff line change
@@ -12,19 +12,21 @@ struct ProductDistribution{N,M,D,S<:ValueSupport,T} <: Distribution{ArrayLikeVar
size::Dims{N}

function ProductDistribution{N,M,D}(dists::D) where {N,M,D}
isempty(dists) && error("product distribution must consist of at least one distribution")
if isempty(dists)
throw(ArgumentError("a product distribution must consist of at least one distribution"))
end
return new{N,M,D,_product_valuesupport(dists),_product_eltype(dists)}(
dists,
_product_size(dists),
)
end
end

function ProductDistribution(dists::AbstractArray{<:Distribution{ArrayLikeVariate{M}},N}) where {M,N}
function ProductDistribution(dists::AbstractArray{<:Distribution{<:ArrayLikeVariate{M}},N}) where {M,N}
return ProductDistribution{M + N,M,typeof(dists)}(dists)
end

function ProductDistribution(dists::NTuple{N,Distribution{ArrayLikeVariate{M}}}) where {M,N}
function ProductDistribution(dists::Tuple{Distribution{<:ArrayLikeVariate{M}},Vararg{Distribution{<:ArrayLikeVariate{M}}}}) where {M}
return ProductDistribution{M + 1,M,typeof(dists)}(dists)
end

@@ -54,10 +56,10 @@ function _product_size(dists::AbstractArray{<:Distribution{<:ArrayLikeVariate{M}
size_dists = size(dists)
return ntuple(i -> i <= M ? size_d[i] : size_dists[i-M], Val(M + N))
end
function _product_size(dists::NTuple{N,Distribution{<:ArrayLikeVariate{M}}}) where {M,N}
function _product_size(dists::Tuple{Distribution{<:ArrayLikeVariate{M}},Vararg{Distribution{<:ArrayLikeVariate{M}}, N}}) where {M,N}
size_d = size(first(dists))
all(size(d) == size_d for d in dists) || error("all distributions must be of the same size")
return ntuple(i -> i <= M ? size_d[i] : N, Val(M + 1))
return ntuple(i -> i <= M ? size_d[i] : N + 1, Val(M + 1))
end

## aliases
@@ -167,7 +169,7 @@ function _rand!(
end

# `_logpdf` for arrays of distributions
# we have to fix a method ambiguity
# we have to fix some method ambiguities
_logpdf(d::ProductDistribution{N}, x::AbstractArray{<:Real,N}) where {N} = __logpdf(d, x)
_logpdf(d::ProductDistribution{2}, x::AbstractMatrix{<:Real}) = __logpdf(d, x)
function __logpdf(
2 changes: 1 addition & 1 deletion src/univariate/discrete/categorical.jl
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@ function pdf(d::Categorical, x::Real)
return insupport(d, x) ? ps[round(Int, x)] : zero(eltype(ps))
end

function _pdf!(r::AbstractArray, d::Categorical{T}, rgn::UnitRange) where {T<:Real}
function _pdf!(r::AbstractArray{<:Real}, d::Categorical{T}, rgn::UnitRange) where {T<:Real}
vfirst = round(Int, first(rgn))
vlast = round(Int, last(rgn))
vl = max(vfirst, 1)
6 changes: 3 additions & 3 deletions src/univariates.jl
Original file line number Diff line number Diff line change
@@ -311,7 +311,7 @@ See also: [`logpdf`](@ref).
pdf(d::UnivariateDistribution, x::Real) = exp(logpdf(d, x))

# extract value from array of zero dimension
_pdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = pdf(d, first(x))
pdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = pdf(d, first(x))

"""
logpdf(d::UnivariateDistribution, x::Real)
@@ -323,7 +323,7 @@ See also: [`pdf`](@ref).
logpdf(d::UnivariateDistribution, x::Real)

# extract value from array of zero dimension
_logpdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = logpdf(d, first(x))
logpdf(d::UnivariateDistribution, x::AbstractArray{<:Real,0}) = logpdf(d, first(x))

# loglikelihood for `Real`
Base.@propagate_inbounds loglikelihood(d::UnivariateDistribution, x::Real) = logpdf(d, x)
@@ -452,7 +452,7 @@ function _pdf_fill_outside!(r::AbstractArray, d::DiscreteUnivariateDistribution,
return vl, vr, vfirst, vlast
end

function _pdf!(r::AbstractArray, d::DiscreteUnivariateDistribution, X::UnitRange)
function _pdf!(r::AbstractArray{<:Real}, d::DiscreteUnivariateDistribution, X::UnitRange)
vl,vr, vfirst, vlast = _pdf_fill_outside!(r, d, X)

# fill central part: with non-zero pdf
21 changes: 21 additions & 0 deletions test/aqua.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using Distributions
using Test

import Aqua

@testset "Aqua" begin
# Test ambiguities separately without Base and Core
# Ref: https://github.com/JuliaTesting/Aqua.jl/issues/77
Aqua.test_all(
Distributions;
ambiguities = false,
# On older Julia versions, installed dependencies are quite old
# Thus unbound type parameters show up that are fixed in newer versions
unbound_args = VERSION >= v"1.6",
)
# Tests are not reliable on older Julia versions and
# show ambiguities in loaded packages
if VERSION >= v"1.6"
Aqua.test_ambiguities(Distributions)
end
end
6 changes: 1 addition & 5 deletions test/runtests.jl
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import JSON
import ForwardDiff

const tests = [
"aqua",
"univariate/continuous/loguniform",
"univariate/continuous/arcsine",
"univariate/discrete/dirac",
@@ -174,8 +175,3 @@ include("testutils.jl")
include("$t.jl")
end
end

# print method ambiguities
println("Potentially stale exports: ")
display(Test.detect_ambiguities(Distributions))
println()