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
Show file tree
Hide file tree
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
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand Up @@ -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/)
Expand Down
1 change: 1 addition & 0 deletions src/censored.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
107 changes: 51 additions & 56 deletions src/common.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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
Expand All @@ -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}

Expand All @@ -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}

Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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}},
Expand Down
6 changes: 3 additions & 3 deletions src/deprecates.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 8 additions & 8 deletions src/mixtures/mixturemodel.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/multivariate/mvtdist.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 8 additions & 6 deletions src/product.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/univariate/discrete/categorical.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/univariates.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
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
Expand Up @@ -11,6 +11,7 @@ import JSON
import ForwardDiff

const tests = [
"aqua",
"univariate/continuous/loguniform",
"univariate/continuous/arcsine",
"univariate/discrete/dirac",
Expand Down Expand Up @@ -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()