0.5-style comprehensions #16622

JeffBezanson · 2016-05-27T23:25:24Z

This could be it! I no longer see a regression in 2d comprehensions. I also implemented @stevengj 's approach, using inference only in the empty case to avoid type inference regressions.

This also makes comprehensions shape-preserving, i.e. [2x for x in rand(2,2)] gives a 2d result.

Todo:

NEWS and docs
rebase after RFC: Change iteratorsize trait of product(itr1, itr2) #16437
tests pass
nanosoldier

yuyichao · 2016-05-27T23:27:16Z

using inference only in the empty case to avoid type inference regressions.

So will there be another breakage in the next (or later) release cycle to return a different type (or raise an error) for empty case?

carnaval · 2016-05-27T23:30:38Z

base/inference.jl

-                end
-            end
-        end
-


tkelman · 2016-05-27T23:34:56Z

I no longer see a regression in 2d comprehensions.

What fixed this part? As of yesterday weren't you thinking you'd have to special-case 2d to avoid a regression?

JeffBezanson · 2016-05-27T23:50:05Z

So will there be another breakage in the next (or later) release cycle

I'm not sure. It might also be ok to give an error in the empty case. That's certainly safest, I just don't have a good handle on how important empty comprehensions are.

What fixed this part?

Not sure; I haven't run the benchmark in 2 weeks. The generated code is definitely really different now. There are many more SROA variables. (test case #15402 (comment))

vtjnash · 2016-05-28T05:15:41Z

test/arrayops.jl

@@ -977,7 +977,7 @@ X = [ i+2j for i=1:5, j=1:5 ]
 @test X[2,3] == 8
 @test X[4,5] == 14
 @test isequal(ones(2,3) * ones(2,3)', [3. 3.; 3. 3.])
-@test isequal([ [1,2] for i=1:2, : ], [1 2; 1 2])
+# @test isequal([ [1,2] for i=1:2, : ], [1 2; 1 2])


add a note that this (undocumented / unused / unknown) syntax is gone now?

Yes, I was just going to bring that up for discussion. Unfortunately there's a bigger problem: this is hitting the infinite-recursion-via-codegen issue --- is there an issue for that? I was just trying to find it.

I'm pretty sure it has an issue number, although I couldn't find it just now either.

Found it: #16201. It was closed.

JeffBezanson · 2016-05-28T05:37:27Z

What to do with the little-acknowledged n-d comprehension syntax, [ [1,2] for i=1:2, : ]? I have a sense it's never used. Easiest to remove it, but I think we can implement it as a function now if necessary.

stevengj · 2016-05-28T12:56:30Z

Is [ [1,2] for i=1:2, : ] even documented? It seems awfully obscure — I'm still not 100% sure what it does even after trying it. Seems better to remove it and see whether people complain.

mschauer · 2016-05-30T09:28:52Z

It prevents comprehensions from becoming vectors of vectors which are awkward to use in julia (compared to arrays), so the functionality is useful.

GunnarFarneback · 2016-05-30T09:36:51Z

Had I known about it I would have used it a couple of times when I have resorted to hcat([some comprehension producing a vector of vectors]...) kind of solutions.

stevengj · 2016-05-30T12:12:14Z

You can always do Vector{Int}[ [1,2] for i=1:2 ]

GunnarFarneback · 2016-05-30T15:00:02Z

Maybe I was unclear. I was talking of the situation of having a comprehension producing a vector of vectors similar to

julia> [[1, 2] for i = 1:3]
3-element Array{Array{Int64,1},1}:
 [1,2]
 [1,2]
 [1,2]

but actually wanting these collected in a matrix, which can be done with

julia> hcat([[1, 2] for i = 1:3]...)'
3x2 Array{Int64,2}:
 1  2
 1  2
 1  2

Had I known about the multidimensional comprehension syntax I would instead have done

julia> [[1, 2] for i = 1:3, :]
3x2 Array{Int64,2}:
 1  2
 1  2
 1  2

for a shorter solution, without splatting a possibly arbitrarily long vector. But I agree that the syntax is obscure and I would be as happy using a function instead.

nalimilan · 2016-05-31T20:54:05Z

I didn't know about that syntax, but I've felt the need for an equivalent of hcat(x...) which doesn't involve splatting. Is there a solution currently?

andreasnoack · 2016-05-31T23:48:16Z

I'm not aware of any so I ended up writing my own in
https://github.com/andreasnoack/TSVD.jl/blob/f777ff95213992c7b7f8daeeb9b4ae8b242f2fc0/src/common.jl#L1-L14

On Tue, May 31, 2016 at 4:54 PM, Milan Bouchet-Valat <
notifications@github.com> wrote:

I didn't know about that syntax, but I've felt the need for an equivalent
of hcat(x...) which doesn't involve splatting. Is there a solution
currently?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#16622 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAe0qeTZEZwnsVITUKkCVsHPg6268GGYks5qHJ_ygaJpZM4Io7vH
.

nalimilan · 2016-06-01T08:49:22Z

Could be good to have something like this in Base.

mschauer · 2016-06-01T10:43:55Z

Basically this can be covered by collecting a Concatenate iterator mentioned in #14805 which would take a generator as argument. The form

Array(concat((1:10 for i = 1:3)))

seems pretty clear and does not create unnecessary intermediates of the ranges in this case.

tkelman · 2016-06-04T01:35:48Z

src/julia-syntax.scm


   'comprehension
   (lambda (e)
-     (expand-forms (lower-comprehension #f       (cadr e) (cddr e))))
+     (if (any (lambda (x) (eq? x ':)) (cddr e))
+         (error "N-d comprehension syntax has been removed"))


only if some of the indices were colons, right?

That's what (if (any (lambda (x) (eq? x ':)) (cddr e)) checks.

I meant the error message should be more specific (I can at least read some scheme)

JeffBezanson · 2016-06-09T16:30:11Z

I believe the only remaining failure here is due to #16201. A bit odd, since the code that hits it (quadgk) doesn't seem to involve comprehensions, but somehow there's an interaction. The issue is fixed by #16692.

JeffBezanson · 2016-06-21T04:07:12Z

Passing CI! Get excited!

vtjnash · 2016-06-21T05:38:02Z

base/inference.jl

    for a in drop(argtypes,1)
        if !(isa(a,Const) || (isType(a) && !has_typevars(a.parameters[1])))
            return false
        end
    end

+    if f === return_type


Just adding a breadcrumb here that I'm still not sure if this is sound. Also, with the way this is currently being used, I expect it will tend to obscure any type errors, meaning this is probably very poorly tested right now.

some generally-applicable changes from #16622

davidagold · 2016-07-04T17:29:51Z

@JeffBezanson on this branch, with X a NullableArray{Float64, 1}, [ f(x) for x in X ] appears to get parsed (is that the right word choice?) into collect(Generator(f, X)), which ends up returning an Array. I want to overload collect so that the aforementioned comprehension returns a NullableArray. Is the ability to do that part of your plan for this PR, or is it not really recommended?

yuyichao · 2016-07-04T17:37:50Z

A comprehension always returns an Array (either on this branch or master). On 0.5, you can use something like NullableArray(f(x) for x in X) to do what you want.

davidagold · 2016-07-04T17:58:06Z

That makes sense. Thanks, sorry for the noise.

quinnj · 2016-07-07T16:16:54Z

Looks like Appveyor was a 32-bit failure related to some kind of GitError. Needs a rebase.

this removes `static_typeof` and `type_goto` fixes #7258

…ents this removes an extra layer of wrapping with `IteratorND` add some inline declarations for `Generator` iteration remove unused `IteratorND` type

this implements the "Steve Johnson compromise": - non-empty comprehensions don't depend on inference at all - in the empty case, we use either Union{} or the inferred type - therefore there is no regression in type precision in cases where we can infer a leaf type add Core.Inference.return_type This computes a return type (at compile time if possible) in a way that handles recursive dependencies in inference.

this also uses the new lowering for typed comprehensions, allowing all comprehensions on unknown-length iterables (fixes #1457)

JeffBezanson · 2016-07-08T20:58:31Z

I've looked at some of the regressions identified by nanosoldier. One issue is #17342.

Another issue is that the comprehension body is not always inlined. If we want, that can (probably) be easily fixed by inserting an :inline node in the function the frontend generates.

Since comprehension expressions tend to be small, it's surprising that they might not be inlined. #17340 has an example of some unexpectedly-large IR from small code, when indexing a LinSpace.

JeffBezanson · 2016-07-11T19:17:21Z

I think the causes of the (few) regressions here are well-understood and we'll be able to address them in the RC period, so I'll merge this now.

ref #17714 [ci skip]

JeffBezanson force-pushed the jb/compr2 branch from b99b339 to 78383da Compare May 27, 2016 23:26

carnaval reviewed May 27, 2016
View reviewed changes

base/inference.jl

end

end

end

Copy link

Contributor

carnaval May 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)

JeffBezanson force-pushed the jb/compr2 branch from 78383da to 59bc754 Compare May 28, 2016 04:54

vtjnash reviewed May 28, 2016
View reviewed changes

mschauer mentioned this pull request Jun 1, 2016

RFC: Concatenating iterator #16708

Closed

stevengj mentioned this pull request Jun 2, 2016

Vectorization Roadmap #16285

Closed

5 tasks

JeffBezanson force-pushed the jb/compr2 branch from 59bc754 to 48967d2 Compare June 3, 2016 16:24

tkelman reviewed Jun 4, 2016
View reviewed changes

JeffBezanson mentioned this pull request Jun 9, 2016

type-inference-independent comprehensions #7258

Closed

JeffBezanson force-pushed the jb/compr2 branch 3 times, most recently from e53df0f to 07a076a Compare June 21, 2016 04:05

vtjnash reviewed Jun 21, 2016
View reviewed changes

yuyichao mentioned this pull request Jun 30, 2016

Add type assertion to Symbol method. Fix #242 JuliaLang/Compat.jl#243

Merged

JeffBezanson added a commit that referenced this pull request Jul 1, 2016

Merge pull request #17214 from JuliaLang/jb/misc

7a95d57

some generally-applicable changes from #16622

JeffBezanson mentioned this pull request Jul 6, 2016

fix LLVM assertion failure on VecElement of aggregate #17292

Merged

JeffBezanson force-pushed the jb/compr2 branch from 4f9e3f8 to f438157 Compare July 6, 2016 04:22

JeffBezanson mentioned this pull request Jul 7, 2016

promote_op/broadcast should not call the function for one(T) #17314

Closed

JeffBezanson mentioned this pull request Jul 7, 2016

n-arg map performance #17321

Closed

JeffBezanson and others added 9 commits July 8, 2016 12:09

implement comprehensions as collect of a Generator

cd35df5

this removes `static_typeof` and `type_goto` fixes #7258

make size of a product iterator concatenate the sizes of its compon…

66f1230

…ents this removes an extra layer of wrapping with `IteratorND` add some inline declarations for `Generator` iteration remove unused `IteratorND` type

do some eta reduction (use f instead of x->f(x)) in Generator syntax

354e9a9

take advantage of shape-preserving comprehensions in vectorize macros

d09f30f

add comprehension filters (fixes #550) and nested loops (fixes #4867)

9bb1423

this also uses the new lowering for typed comprehensions, allowing all comprehensions on unknown-length iterables (fixes #1457)

doc/NEWS updates for comprehension changes

74f5c03

Tests and documentation for generators with guards and nesting.

4fe43d0

shorten call chain for collect(::Generator)

b363cc7

JeffBezanson force-pushed the jb/compr2 branch from f438157 to b363cc7 Compare July 8, 2016 18:39

JeffBezanson mentioned this pull request Jul 8, 2016

Another unnecessary GC root #17342

Closed

JeffBezanson merged commit 0572533 into master Jul 11, 2016

tkelman deleted the jb/compr2 branch July 12, 2016 08:10

davidagold mentioned this pull request Jul 12, 2016

map revisions JuliaStats/NullableArrays.jl#128

Merged

schmrlng mentioned this pull request Jul 18, 2016

Document recursive (c)transpose #15795

Closed

davidagold mentioned this pull request Aug 6, 2016

Discussion of current dplyr and LINQ like APIs JuliaData/DataFrames.jl#1025

Closed

zuo referenced this pull request Sep 4, 2016

NEWS updates for Dict comprehensions and map of Dict.

fc3cbc9

ref #17714 [ci skip]

JoshChristie mentioned this pull request Sep 21, 2016

julia 0.5.0 package errors rened/DeclarativePackages.jl#12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

0.5-style comprehensions #16622

0.5-style comprehensions #16622

JeffBezanson commented May 27, 2016 •

edited

Loading

yuyichao commented May 27, 2016

carnaval May 27, 2016

tkelman commented May 27, 2016

JeffBezanson commented May 27, 2016 •

edited

Loading

vtjnash May 28, 2016

JeffBezanson May 28, 2016

vtjnash May 28, 2016

JeffBezanson May 28, 2016

JeffBezanson commented May 28, 2016

stevengj commented May 28, 2016 •

edited

Loading

mschauer commented May 30, 2016 •

edited

Loading

GunnarFarneback commented May 30, 2016

stevengj commented May 30, 2016

GunnarFarneback commented May 30, 2016

nalimilan commented May 31, 2016

andreasnoack commented May 31, 2016

nalimilan commented Jun 1, 2016

mschauer commented Jun 1, 2016

tkelman Jun 4, 2016

JeffBezanson Jun 9, 2016

tkelman Jun 9, 2016 •

edited

Loading

JeffBezanson commented Jun 9, 2016 •

edited

Loading

JeffBezanson commented Jun 21, 2016

vtjnash Jun 21, 2016

davidagold commented Jul 4, 2016

yuyichao commented Jul 4, 2016

davidagold commented Jul 4, 2016

quinnj commented Jul 7, 2016

JeffBezanson commented Jul 8, 2016

JeffBezanson commented Jul 11, 2016

0.5-style comprehensions #16622

0.5-style comprehensions #16622

Conversation

JeffBezanson commented May 27, 2016 • edited Loading

yuyichao commented May 27, 2016

Choose a reason for hiding this comment

tkelman commented May 27, 2016

JeffBezanson commented May 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented May 28, 2016

stevengj commented May 28, 2016 • edited Loading

mschauer commented May 30, 2016 • edited Loading

GunnarFarneback commented May 30, 2016

stevengj commented May 30, 2016

GunnarFarneback commented May 30, 2016

nalimilan commented May 31, 2016

andreasnoack commented May 31, 2016

nalimilan commented Jun 1, 2016

mschauer commented Jun 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman Jun 9, 2016 • edited Loading

Choose a reason for hiding this comment

JeffBezanson commented Jun 9, 2016 • edited Loading

JeffBezanson commented Jun 21, 2016

Choose a reason for hiding this comment

davidagold commented Jul 4, 2016

yuyichao commented Jul 4, 2016

davidagold commented Jul 4, 2016

quinnj commented Jul 7, 2016

JeffBezanson commented Jul 8, 2016

JeffBezanson commented Jul 11, 2016

JeffBezanson commented May 27, 2016 •

edited

Loading

JeffBezanson commented May 27, 2016 •

edited

Loading

stevengj commented May 28, 2016 •

edited

Loading

mschauer commented May 30, 2016 •

edited

Loading

tkelman Jun 9, 2016 •

edited

Loading

JeffBezanson commented Jun 9, 2016 •

edited

Loading