@parallel for evaluates its range twice #8207
Closed
lucasb-eyer opened this Issue on Sep 1, 2014 · 6 comments
Projects
None yet
Labels
parallel
Milestone
No milestone
Assignees
No one assigned
3 participants
@lucasb-eyer
@elextr
@JeffBezanson
Notifications

You’re not receiving notifications from this thread.
@lucasb-eyer
lucasb-eyer commented on Sep 1, 2014

Edit: better example below.

The following fails

res = @parallel (a,b)->"$a\n$b" for line in readlines()
    "$line"
end

with this incomprehensible error message:

exception on 1: ERROR: BoundsError()
BoundsError()

While this works:

lines = readlines()
res = @parallel (a,b)->"$a\n$b" for line in lines
    "$line"
end

I looked into this and I think @parallel should generate a statement that assigns r to a local variable and use that throughout. I tried to fix it, but I'm only a lvl1 macro wizard and those localize_vars killed me.

As a sidenote, this raises the question of how -if at all- to handle "unrepeatable" iterables such as eachline() which will still fail after this fix. I've always wanted to be able to programatically tell whether an iterable is "repeatable" or not in python.
@elextr
elextr commented on Sep 1, 2014

readlines() is not a parallelizable function.

I would have thought that in general detecting non-parallelizable expressions in the for is difficult enough to make such solutions hard to generalize.

Your second example also has the benefit that it is explicit that the whole file must be read before passing it to the loop.

A better error message would however be good.
@lucasb-eyer
lucasb-eyer commented on Sep 1, 2014

I wasn't thinking about parallelizing readlines() itself, but parallelizing the iteration over its result, which is an array. AFAICT, the problem here is that the expression bound to r in @parallel will be evaluated twice in the generated code, instead of being evaluated once and the result of it used twice. Here is a better example of what I mean:

a = @parallel (+) for i in (println("hi"); 1:5)
    i
end

It will print "hi" twice, while I'd only expect it once.
@lucasb-eyer lucasb-eyer changed the title from @parallel for readlines() to @parallel for evaluates its range twice on Sep 1, 2014
@elextr
elextr commented on Sep 1, 2014

Sorry, I said that badly, I certainly wasn't suggesting parallelizing readlines().

A better way of saying it is that the range expression of the for is meant to be something that can be subdivided to select the values in the body expression, see splitrange. Obviously the canonical example is a range :)
@JeffBezanson JeffBezanson added the parallel label on Sep 1, 2014
@lucasb-eyer
lucasb-eyer commented on Sep 5, 2014

I do understand, but what I'm arguing/hoping for is the result of the expression to meet this condition, not the expression itself.

macroexpand(:(@parallel for i in (println("hi"); 1:5) i end))
quote  # multi.jl, line 1488:
    Base.pfor($(Expr(:localize, :(()->begin  # expr.jl, line 96:
            begin  # multi.jl, line 1449:
                function (#125#lo::Base.Int,#126#hi::Base.Int) # multi.jl, line 1450:
                    for i = (begin 
                                println("hi")
                                1:5
                            end)[#125#lo:#126#hi] # line 1451:
                        begin  # line 2:
                            i
                        end
                    end
                end
            end
        end))),Base.length(begin 
                println("hi")
                1:5
            end))
end

should not use the whole expression twice. Rather, I'd hope for it to evaluate the expression, store its result and use that twice. Something like (I'm inventing this):

quote  # multi.jl, line 1488:
    the_range = (begin 
        println("hi")
        1:5
    end)
    Base.pfor($(Expr(:localize, :(()->begin  # expr.jl, line 96:
        begin  # multi.jl, line 1449:
                function (#125#lo::Base.Int,#126#hi::Base.Int) # multi.jl, line 1450:
                    for i = the_range[#125#lo:#126#hi] # line 1451:
                        begin  # line 2:
                            i
                        end
                    end
                end
            end
        end))),Base.length(the_range))
end

wouldn't that be viable?
@elextr
elextr commented on Sep 5, 2014

Looks ok to me (in principle) but, like you, my macro fu doesn't extend to making the_range have the right scoping.
@JeffBezanson JeffBezanson added a commit that closed this issue on Aug 21, 2015
	@JeffBezanson 	fix #8207, `@parallel` evaluates its range twice 			30ed04c
@JeffBezanson JeffBezanson closed this in 30ed04c on Aug 21, 2015
@lucasb-eyer
lucasb-eyer commented on Aug 22, 2015

Nice, thanks!