ahwillia / Einsum.jl

Einstein summation notation in Julia
MIT License
151 stars 17 forks source link

Support for qualified function names #17

Closed dfdx closed 7 years ago

dfdx commented 7 years ago

On Julia 0.5:

x = rand(3)
inc(x) = x .+ 1

@einsum y[i] := inc(x[i])  # ==> ok
@einsum y[i] := Main.inc(x[i])  # ==> ERROR: syntax: malformed expression

It turns out the function call is transformed into Main.(inc)(x[i]) which is exactly the source of the error. Oddly enough, pure _einsum() doesn't have this issue and generates absolutely correct code.

It's also worth to mention that on Julia 0.4 the error is different:

ERROR: TypeError: getfield: expected Symbol, got Function in anonymous at /home/slipslop/.julia/v0.5/Einsum/src/Einsum.jl:217

dfdx commented 7 years ago

Further investigation shows that the issue is caused by the call to remove_quote_nodes!(ex): if I comment out this line, code work fine. However, in REPL I don't see that remove_quote_nodes! changes the expression in any way.

I definitely don't understand something about macro expansion in Julia.

dfdx commented 7 years ago

I've got some more time to work on this issue. Now I see that remove_quote_nodes! really corrupts composed expressions like Main.inc. Without remove_quote_nodes! we have expressions like this:

julia> Expr(:., :Main, QuoteNode(:inc))
 :(Main.inc)

with it we get:

julia> Expr(:., :Main, :inc)
 :(Main.(inc))

which is invalid syntax.

So I should ask: why was this function introduced? Can we replace it with something with the same intended behavior, but without these unfortunate side effects?

cc: @ahwillia

ahwillia commented 7 years ago

I'm happy for you to change this function as long as it doesn't break any of the tests that are already in place! The problem is that certain parts of the expression are QuoteNodes, not Expressions, which are slightly different representations in Julia.

Unfortunately, I don't have time to address this very soon. But please do play around and I'll review a PR if you get it working.

dfdx commented 7 years ago

Just wanted to let you know that I fixed it and am just waiting for better internet connection to make a PR.