Celeste.jl icon indicating copy to clipboard operation
Celeste.jl copied to clipboard

types for sources' parameters; no more `ids`

Open jeff-regier opened this issue 7 years ago • 2 comments

Currently we have to use the right ids variables to index into vectors of sources' parameters. And we can't use Julia's type system to dispatch methods that convert between different types of parameter vectors. The code below shows 1) how to define a typed vector of parameters, and 2) that doing so doesn't effect runtime. For this issue, we'd replace the current parameter vectors with SourceParams.

immutable ParamIndexes{T} end

abstract ParamSet
immutable MeanField <: ParamSet end

immutable SourceParams{ParamType <: ParamSet, NumType <: Number}
    val::Vector{NumType}
end

import Base.setindex!
import Base.getindex

getindex(sp::SourceParams, ::ParamIndexes{:u}) = sp.val[1:2]
setindex!(sp::SourceParams, val, ::ParamIndexes{:u}) = (sp.val[1:2] = val)
getindex(sp::SourceParams, ::ParamIndexes{:e_dev}) = sp.val[3]
setindex!(sp::SourceParams, val, ::ParamIndexes{:e_dev}) = (sp.val[3] = val)

function speed_test()
    u = ParamIndexes{:u}()
    e_dev = ParamIndexes{:e_dev}()

    sp = SourceParams{MeanField, Float64}(zeros(32))
    new_u = randn(2)

    @inbounds for i in 1:10_000_000
        sp[e_dev] += 9.
    end

    a = 0.
    for i in 1:10_000_000
        a += 9.
    end

    a, val, sp
end

speed_test()
@time speed_test()

jeff-regier avatar Oct 04 '16 20:10 jeff-regier

This seems much nicer than the current approach. One could simplify this slightly using the Val type from Base instead of ParamIndexes: define, e.g. getindex(sp::SourceParams, ::Type{Val{:u}}), and index like sp[Val{:u}]. Terser than sp[ParamIndexes{:u}()].

I assume there is some reason (possibly performance) that we want all parameters in a contiguous vector in memory, precluding a definition like

struct SourceParams{..., T<:Number}
    u::Vector{T}
    e_dev::Vector{T}
    ...
end

?

kbarbary avatar Sep 10 '17 22:09 kbarbary

Correct, we want to keep the parameters contiguous in memory. @keno has a branch of Celeste that does away this ids, I think. I believe he used function calls to get the relevant ranges of the parameters vector.

@keno What branch is it? Can it be merged?

jeff-regier avatar Sep 10 '17 23:09 jeff-regier