JuliaDynamics / ResumableFunctions.jl

C# style generators a.k.a. semi-coroutines for Julia.
Other
158 stars 19 forks source link

odd issue with comprehensions #19

Closed jverzani closed 6 years ago

jverzani commented 6 years ago

I ran into a surprise for me in trying something like this:

@resumable function test3()

    for u in [[(1,2),(3,4)], [(5,6),(7,8)]]
        for i in 1:2
            val = [a[i] for a in u]
            @yield val      
        end
    end

end

collect(test3()) # errors -- ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Core.Box

Whereas writing out the loop as follows is fine:


@resumable function test2()

    for u in [[(1,2),(3,4)], [(5,6),(7,8)]]
        val = [a[1] for a in u]
        @yield val      
        val = [a[2] for a in u]
        @yield val      
    end

end
collect(test2())

I don't have insight why there is a distinction, and can work around in my usage, but thought it might be of interest to point out.

BenLauwens commented 6 years ago

Hi

I looked at your code and the @resumable macro works without bug... but if we lower the function:

function test3()
  for u in [[(1,2),(3,4)], [(5,6),(7,8)]]
    for i in 1:2
      val = [a[i] for a in u]
      return val
    end
  end
end

we get:

code_warntype(test3,())
Variables:
  #self# <optimized out>
  #31::##31#32
  i::Core.Box
  val::Array{_,1} where _
  #temp#@_5::Int64
  u::Array{Tuple{Int64,Int64},1}
  #temp#@_7::Int64

Body:
  begin
      SSAValue(0) = $(Expr(:invoke, MethodInstance for vect(::Array{Tuple{Int64,Int64},1}, ::Vararg{Array{Tuple{Int64,Int64},1},N} where N), :(Base.vect), :($(Expr(:invoke, MethodInstance for vect(::Tuple{Int64,Int64}, ::Vararg{Tuple{Int64,Int64},N} where N), :(Base.vect), (1, 2), (3, 4)))), :($(Expr(:invoke, MethodInstance for vect(::Tuple{Int64,Int64}, ::Vararg{Tuple{Int64,Int64},N} where N), :(Base.vect), (5, 6), (7, 8))))))
      #temp#@_7::Int64 = 1
      3:
      unless (Base.not_int)((#temp#@_7::Int64 === (Base.add_int)((Base.arraylen)(SSAValue(0))::Int64, 1)::Int64)::Bool)::Bool goto 28
      SSAValue(7) = (Base.arrayref)(SSAValue(0), #temp#@_7::Int64)::Array{Tuple{Int64,Int64},1}
      SSAValue(8) = (Base.add_int)(#temp#@_7::Int64, 1)::Int64
      u::Array{Tuple{Int64,Int64},1} = SSAValue(7)
      #temp#@_7::Int64 = SSAValue(8) # line 4:
      SSAValue(9) = (Base.select_value)((Base.sle_int)(1, 2)::Bool, 2, (Base.sub_int)(1, 1)::Int64)::Int64
      #temp#@_5::Int64 = 1
      12:
      unless (Base.not_int)((#temp#@_5::Int64 === (Base.add_int)(SSAValue(9), 1)::Int64)::Bool)::Bool goto 26
      i::Core.Box = $(Expr(:new, :(Core.Box)))
      SSAValue(10) = #temp#@_5::Int64
      SSAValue(11) = (Base.add_int)(#temp#@_5::Int64, 1)::Int64
      SSAValue(4) = SSAValue(10)
      (Core.setfield!)(i::Core.Box, :contents, SSAValue(4))::Int64
      #temp#@_5::Int64 = SSAValue(11) # line 5:
      #31::##31#32 = $(Expr(:new, :(Main.##31#32), :(i)))
      SSAValue(6) = $(Expr(:new, Base.Generator{Array{Tuple{Int64,Int64},1},##31#32}, :(#31), :(u)))
      val::Array{_,1} where _ = $(Expr(:invoke, MethodInstance for collect(::Base.Generator{Array{Tuple{Int64,Int64},1},##31#32}), :(Base.collect), SSAValue(6))) # line 6:
      return val::Array{_,1} where _
      26:
      goto 3
      28:
      return
  end::Union{Array{_,1} where _, Void}

i is a Core.Box and this is what the macro @resumable uses. I don't understand why? Perhaps a compiler bug?

jverzani commented 6 years ago

Thanks for looking. I wouldn't know what to point to for a useful bug report. Please do if you have thoughts.

BenLauwens commented 6 years ago

I forwarded this issue to Julia Discourse...

BenLauwens commented 6 years ago

Apparently this is a known bug in Julia. A not so nice solution is to use a let statement to help the compiler. But the macro @resumable can not deal with a @yield inside a let statement... so I think you have to use your backup solution... Sorry

jverzani commented 6 years ago

Thanks for investigating and the great package!

On Sun, Feb 25, 2018 at 9:32 AM, Ben Lauwens notifications@github.com wrote:

Apparently this is a known bug in Julia. A not so nice solution is to use a let statement to help the compiler. But the macro @resumable can not deal with a @yield inside a let statement... so I think you have to use your backup solution... Sorry

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BenLauwens/ResumableFunctions.jl/issues/19#issuecomment-368314006, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZvTN9tMhOV9vyzWOCmD2MbYfJ-rcDGks5tYW72gaJpZM4SSKUl .

garrison commented 6 years ago

Link to discourse thread: https://discourse.julialang.org/t/index-inside-a-comprehension-is-boxed/9315

BenLauwens commented 6 years ago

Try the branch Python-generators-v0.6. A let block can be used to fix this issue:

@resumable function test_let()
  for u in [[(1,2),(3,4)], [(5,6),(7,8)]]
    for i in 1:2
      let i=i
        val = [a[i] for a in u]
      end
      @yield val
    end
  end
end

The variables declared in a let block are local and are not restored after the @yield. This solves the problem!

jverzani commented 6 years ago

Great! Thanks so much for following up on this.