add loglogistic distribution #1780
Conversation
Codecov ReportAttention:
... and 10 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
|
This looks like a pretty solid implementation, but I do wonder if we should be adding Log* distributions piecemeal like this, rather than providing a more general interface for transformed variables. |
It sounds like a very interesting idea to provide a general interface for transformed variables. But I feel there might be some difficulties. 1) commonly used parameterization might not be consistent for distribution on |
|
|
||
| ```julia | ||
| LogLogistic() # Log-logistic distribution with unit scale and unit shape | ||
| LogLogistic(θ) # Log-logistic distribution with scale θ and unit shape |
There was a problem hiding this comment.
I'm concerned calling theta the "scale" parameter might confuse users, since the scale parameter is exp(location) in this case.
There was a problem hiding this comment.
Thank you for the PR!
I haven't checked the math yet but commented on a few Julia issues that we should fix before merging the PR.
It's good that you added tests! We already have quite extensive automatic tests for distributions but that requires that you add reference values using R to test/refs. Can you fix that?
| @distr_support LogLogistic 0.0 Inf | ||
|
|
||
| #### Coversions | ||
| convert(::Type{LogLogistic{T}}, θ::S, ϕ::S) where {T <: Real, S <: Real} = LogLogistic(T(θ), T(ϕ)) |
There was a problem hiding this comment.
The constructor already exists for this purpose:
| convert(::Type{LogLogistic{T}}, θ::S, ϕ::S) where {T <: Real, S <: Real} = LogLogistic(T(θ), T(ϕ)) |
|
|
||
| #### Coversions | ||
| convert(::Type{LogLogistic{T}}, θ::S, ϕ::S) where {T <: Real, S <: Real} = LogLogistic(T(θ), T(ϕ)) | ||
| convert(::Type{LogLogistic{T}}, d::LogLogistic{S}) where {T <: Real, S <: Real} = LogLogistic(T(d.θ), T(d.ϕ), check_args=false) |
There was a problem hiding this comment.
This is not completely correct as it does not guarantee that you get the desired type, and it's missing the case when the type is already correct:
| convert(::Type{LogLogistic{T}}, d::LogLogistic{S}) where {T <: Real, S <: Real} = LogLogistic(T(d.θ), T(d.ϕ), check_args=false) | |
| convert(::Type{LogLogistic{T}}, d::LogLogistic{T}) where {T<:Real} = d | |
| convert(::Type{LogLogistic{T}}, d::LogLogistic) where {T<:Real} = LogLogistic{T}(T(d.θ), T(d.ϕ)) |
| if d.ϕ ≤ 1 | ||
| error("mean is defined only when ϕ > 1") | ||
| end | ||
| return d.θ*π/d.ϕ/sin(π/d.ϕ) |
There was a problem hiding this comment.
You might want to use
| return d.θ*π/d.ϕ/sin(π/d.ϕ) | |
| return d.θ/sinc(1/d.ϕ) |
| #### Evaluation | ||
| function pdf(d::LogLogistic, x::Real) | ||
| if x ≤ zero(0) | ||
| z = zero(x) |
There was a problem hiding this comment.
This creates a type instability, generally the two branches yield z of different types.
|
|
||
| function var(d::LogLogistic) | ||
| if d.ϕ ≤ 2 | ||
| erros("var is defined only when ϕ > 2") |
|
|
||
| function logcdf(d::LogLogistic, x::Real) | ||
| if x <= 0 | ||
| -Inf |
There was a problem hiding this comment.
I guess it was supposed to be
return -InfBut that would again create a type instability.
This PR adds log-logistic distribution to the package.
The log-logistic distribution has two parameters: scale and shape. It has been widely used in survival analysis.
The model implementation is in
src/univariate/continuous/loglogistic.jlThe test is in
test/univariate/continuous/loglogistic.jl