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

Proposal: generalise logsumexp slightly. #69

Closed
wants to merge 1 commit into from
Closed

Proposal: generalise logsumexp slightly. #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions src/basicfuns.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,13 @@ function logsumexp(X)
isempty(X) && return log(sum(X))
reduce(logaddexp, X)
end
function logsumexp(X::AbstractArray{T}) where {T<:Real}
function logsumexp(X::AbstractArray{T}; dims=nothing) where {T<:Real}
dims isa Nothing && return _logsumexp(X)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more contentional to make this something like

Suggested change
dims isa Nothing && return _logsumexp(X)
dims === nothing && return _logsumexp(X)

though it doesn't really matter; they're entirely equivalent and should perform the same. Just figured I'd note it.

Copy link
Contributor

@cossio cossio May 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnothing(dims)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=== is special cased in the compiler and is generally more efficient for checking nothing (and indeed, even missing).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think isnothing is also elided since it's dealt with at dispatch time, and it was the recommended way to check for nothingness? The only thing is that it requires Julia 1.1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was the recommended way to check for nothingness?

How did you come to that conclusion?

I think isnothing is also elided

It is not, see JuliaLang/julia#27681.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you come to that conclusion?

Well, it's a exported function named isnothing :trollface:
In reality I wasn't aware of that issue. Thanks for pointing it out.

isempty(X) && return log(zero(T))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this type unstable? I think you'd still have to consider dimensions as in

julia> sum(zeros(0, 0), dims=1)
1×0 Array{Float64,2}

u = maximum(X; dims=dims)
return log.(sum(exp.(X .- u); dims=dims)) .+ u
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe reuse the array created by sum(exp.(X .- u); dims=dims) to avoid a temporary.

end
function _logsumexp(X::AbstractArray{T}) where {T<:Real}
isempty(X) && return log(zero(T))
u = maximum(X)
isfinite(u) || return float(u)
Expand All @@ -199,7 +205,6 @@ function logsumexp(X::AbstractArray{T}) where {T<:Real}
end
end


"""
softmax!(r::AbstractArray, x::AbstractArray)

Expand Down
3 changes: 3 additions & 0 deletions test/basicfuns.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ end
@test logsumexp(arguments) ≡ result
end
end

x = randn(5, 1)
@test logsumexp(x) == logsumexp(x; dims=1)[1]
end

@testset "softmax" begin
Expand Down