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

Make LMArray labels fast #2

Closed
ChrisRackauckas opened this issue Oct 20, 2017 · 3 comments
Closed

Make LMArray labels fast #2

ChrisRackauckas opened this issue Oct 20, 2017 · 3 comments

Comments

@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented Oct 20, 2017

I was hoping

Base.@pure function symbol_to_index(names::SArray, s::Symbol)
    findfirst((t)->s==t,names)
end

or

Base.@pure function symbol_to_index(names, s::Symbol)
    i = 0
    for n in names
        i += 1
        n == s && return i
    end
    return error("Label not valid")
end

would make

@inline function Base.getindex(A::LMArray{S,T,N,L}, s::Symbol) where {L,N,T,S<:Tuple}
    A.values[symbol_to_index(A.names, s)]
end

the symbol_to_index call be essentially free after the first time it compiles and basically makes a map between symbols and indices, but that doesn't seem to be the case. Do I need to Val{s}? That would be fine, especially after JuliaLang/julia#1974 .

@mbauman @timholy can I call on your expertise for some advice here on pure functions? There must be some way to get Julia to know that the names are constant in some form and optimize on that for literals.

@ChrisRackauckas
Copy link
Member Author

Looks like JuliaLang/julia#24441 will fix this.

Refs:

JuliaLang/julia#24011
JuliaLang/julia#24441

@mbauman
Copy link

mbauman commented Jan 3, 2018

Did you figure this one out? I don't think a @pure function is allowed to throw errors, since you're telling the compiler it's safe to execute at any time, including in inference and compilation.

@ChrisRackauckas
Copy link
Member Author

I'm going to wait until v0.7 to see if any of the inference changes help. I think the correct interface for this is via getfield overloading, in which case that should be fast I think if it optimizes on the constant correctly. That or IPO should do the trick.

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

No branches or pull requests

2 participants