JuliaSIMD / Polyester.jl

The cheapest threads you can find!
MIT License
244 stars 14 forks source link

Undefined nested iterating variable #108

Closed weymouth closed 7 months ago

weymouth commented 1 year ago

This simple example throws an error

using Polyester
x = collect(1:12)
y = zeros(6)
@batch for i in eachindex(y)
    y[i] = sum(x[j] for j in 2i-oneunit(i):2i)
end
y

ERROR: UndefVarError: j not defined

I can redefine the loop to get rid of the variable j in this case, but shouldn't this work in principle?

chriselrod commented 1 year ago

but shouldn't this work in principle?

Yes, Polyester doesn't realize j is defined by your generator, and thinks it is a variable you're passing into/using inside your loop. You can modify the code inside src/closure.jl to recognize the generator. https://github.com/JuliaSIMD/Polyester.jl/blob/5185467b7394df6c483d7212f83582baab0106d7/src/closure.jl#L94

weymouth commented 1 year ago

I'm terrible at macros, so I will personally not be diving into that. ;-)

weymouth commented 1 year ago

I found that blocking the closure into it's own function is a general workaround. ie

using Polyester
x = collect(1:12)
y = zeros(6)
closure(i,x) = sum(x[j] for j in 2i-oneunit(i):2i)
@batch for i in eachindex(y)
    y[i] = closure(i,x)
end
y
chriselrod commented 1 year ago

Does that work on ARM? This is still a Polyester bug.

weymouth commented 1 year ago

I don't have a computer with an ARM, so I can't check, sorry. The github CLI tests all passed but I'm not sure if any of those are ARM.

chriselrod commented 1 year ago

Nevermind, seems to work fine on ARM.

Still, this should be left open because workarounds or not, it is a real issue with the package.

SouthEndMusic commented 11 months ago

I found a similar problem when trying to loop over an iterable of functions:

using Polyester

functions = [x -> n*x for n in 1:3]
data = rand(100)

@batch for i in eachindex(data)
   for f in functions
      data[i] += f(data[i])
   end
end
# Yields 'ERROR: UndefVarError: `f` not defined'