JuliaRobotics / KernelDensityEstimate.jl

Kernel Density Estimate with product approximation using multiscale Gibbs sampling
GNU Lesser General Public License v2.1
23 stars 7 forks source link

Native Integer Size #19

Closed Affie closed 6 years ago

Affie commented 6 years ago

Throughout the code integers are represented as Int64. This causes problems on 32 bit machines for example

    n::Int64
    reshape(M, n, 1)

The 1 is interpreted as a Int32 giving an error of no method found for reshape(::Array, ::Int64, ::Int32) I would suggest using Int as it represents either Int32 or Int64 depending on native format. Of course only where Int32 is large enough to represent the number, arrays of length 2e9 might take a bit long to compute on an small processor in any case :-) I changed it as a test to get KernelDensityEstimates running on a raspberry pi

dehann commented 6 years ago

Looks like we can use the Signed type which is Union{Int32, Int64}. I'm concerned that database interactions or other middleware communication might force an Int64 on ARM. I think we should just work with Signed.

using BenchmarkTools

function ff(i::T) where {T <: Signed}
   i+0
end
function fi(i::Int)
   i+0
end
# compile functions
ff(Int32(1))
fi(Int32(1))
ones(Int64, 10);
ones(Int32,10);
@btime 1

julia> @btime ff.(ones(Int32,10000));
  7.263 μs (4 allocations: 117.34 KiB)
julia> @btime ff.(ones(Int64,10000));
  9.937 μs (4 allocations: 156.41 KiB)
julia> @btime fi.(ones(Int64,10000));
  9.925 μs (4 allocations: 156.41 KiB)
Affie commented 6 years ago

As an example, one of the errors would be here

https://github.com/dehann/KernelDensityEstimate.jl/blob/e085760c62e43a8ac421aecc0515cbabf7b533fc/src/BallTreeDensity01.jl#L197

Np, Nd is of type Int64, the one in reshape is of type Int32 on a 32 bit system. therefore, no method reshape is found

dehann commented 6 years ago

see b2615e5 for converting all Int64 to just Int