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

Modify the nansum and nanmean function #15

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Modify the nansum and nanmean function #15

merged 1 commit into from
Apr 19, 2023

Conversation

Internal-Tide
Copy link
Contributor

@Internal-Tide Internal-Tide commented Apr 19, 2023

The nansum and nanmean functions are no need to copy, it will slow down the code.
This is added new function

function nanmean(x::AbstractArray{T}) where {T<:Number}
    # n = length(x)
    sum = zero(T)
    count = 0
    for i in eachindex(x)
        if !isnan(x[i])
            sum += x[i]
            count += 1
        end
    end
    return sum / count
end

function nanmean(x::AbstractArray{T}, dims) where {T<:Number}
    return mapslices(nanmean, x; dims=dims)
end


function nansum(arr::AbstractArray{T}) where {T<:Number}
    return (sum(x -> !isnan(x) * x, arr))
end

function nansum(arr::AbstractArray{T}, dims) where {T<:Number}
    return (sum(x -> !isnan(x) * x, arr, dims=dims))
end

The speed test is:

using PhysOcean
using BenchmarkTools
using Random

seed = Random.seed!(1234)
a = rand(seed, [NaN, 1.0, 2.5, 0.11, 20.0], (1000, 1000))
@btime nansum($a, 1)
@btime PhysOcean.nansum($a, 1)
@btime nanmean($a, 1)
@btime PhysOcean.nanmean($a, 1)
result: 
110.660 μs (1 allocation: 7.94 KiB)
1.648 ms (11 allocations: 9.29 MiB)
1.740 ms (9501 allocations: 243.50 KiB)
1.889 ms (16 allocations: 9.42 MiB)

The new nansum function is much faster!
However, when the array size is small, the old function is faster than new function, but when the array size is small, I don't think everyone is sensitive to 100 or 200 ns.

@Alexander-Barth
Copy link
Member

Thanks a lot!

@Alexander-Barth Alexander-Barth merged commit 22f45eb into gher-uliege:master Apr 19, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants