SciML / ComponentArrays.jl

Arrays with arbitrarily nested named components.
MIT License
296 stars 35 forks source link

Type inference with getproperty #47

Closed dcabecinhas closed 4 years ago

dcabecinhas commented 4 years ago

I'm trying to index into a CA with a symbol but I get type inference issues.

It looks to me that f() and h() should be equivalent but somehow f() is not being infered.

Is this related with #9? Is there a way around this issue?

julia> ca = ComponentArray(a=1,b=2)
ComponentVector{Int64}(a = 1, b = 2)

julia> s = :a
:a

julia> function f(ca,s)
         getproperty(ca,s)
       end
f (generic function with 1 method)

julia> function g(ca)
         ca.a
       end
g (generic function with 1 method)

julia> function h(ca)
         getproperty(ca,:a)
       end
h (generic function with 1 method)

julia> @code_warntype f(ca,s)
Variables
  #self#::Core.Compiler.Const(f, false)
  ca::ComponentVector{Int64}
  s::Symbol

Body::Any
1 ─ %1 = Main.getproperty(ca, s)::Any
└──      return %1

julia> @code_warntype g(ca)
Variables
  #self#::Core.Compiler.Const(g, false)
  ca::ComponentVector{Int64}

Body::Int64
1 ─ %1 = Base.getproperty(ca, :a)::Int64
└──      return %1

julia> @code_warntype h(ca)
Variables
  #self#::Core.Compiler.Const(h, false)
  ca::ComponentVector{Int64}

Body::Int64
1 ─ %1 = Main.getproperty(ca, :a)::Int64
└──      return %1
jonniedie commented 4 years ago

Yeah, this is related to that #9. Honestly, I still have no idea how to get the constants to propagate here. I'm not sure there's even much I can do. It's deep compiler magic.

But there is a workaround. Use s = Val(:a). Although for some reason, I only have it set up to work with getindex and not getproperty, even though they should be the same. I'll push an update with the new method for getproperty.

jonniedie commented 4 years ago

It's kinda a bummer because using Val(:a) doesn't really help when you need to get properties with dynamically changing symbols since it just kicks the problem over to the Val call. But I don't really know that it's a solvable problem for that case, since what you would be doing there is inherently type unstable.

With that said, I think I'm going to close this one as a duplicate of #9 because that's really the root of the problem.

dcabecinhas commented 4 years ago

Thanks. Val solves my use-case.

Ideally constant-propagation should work but generated functions are not compiler friendly :/

jonniedie commented 4 years ago

On a related note, I needed to iterate over components of a ComponentVector today and realized that this was always going to be super slow because the compiler can't infer what comes out of the call keys(ca). So now there is a new function in the update I just pushed (a09b9a4, version 0.8.0) called valkeys that returns Val-wrapped keys so you can iterate over subcomponents in a fast and type-stable way.

  julia> using ComponentArrays, BenchmarkTools

  julia> ca = ComponentArray(a=1, b=[1,2,3], c=(a=4,))
  ComponentVector{Int64}(a = 1, b = [1, 2, 3], c = (a = 4))

  julia> @btime sum(prod($ca[k]) for k in keys($ca))
    9.099 μs (7 allocations: 176 bytes)
  11

  julia> @btime sum(prod($ca[k]) for k in valkeys($ca))
    11.511 ns (0 allocations: 0 bytes)
  11