Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is @tuplegen broken? #13

Open
btmit opened this issue Mar 18, 2024 · 11 comments
Open

Is @tuplegen broken? #13

btmit opened this issue Mar 18, 2024 · 11 comments

Comments

@btmit
Copy link

btmit commented Mar 18, 2024

using Unroll

function foo()
    # example in docs
    v = @tuplegen [(i==2) ? i * 6 : i for i = 1 : 4]
    return v
end

ERROR: LoadError: Expression following tuplegen macro must be a comprehension as described in the documentation

With a little digging I was able to confirm that length(expr.args) == 1 instead of 2 which is what triggers the error. I'm not familiar enough with macros to dig deeper than that. Maybe I'm using it wrong, but I've been able to get @unroll working just fine.

@StephenVavasis
Copy link
Owner

I am the original author of this package, but I stopped maintaining it many years ago. This package is difficult to maintain because it relies on the undocumented aspects of internal representations of expressions, which can change from one release of the compiler to the next. If you would like to try to fix the problem yourself, please feel free to submit a PR. To get started, you can use the dump function on expressions to see the internal representation. Maybe you can figure out why the internal representation of comprehensions in recent versions of Julia differs is incompatible with my old code. Here is an example of how to see an internal representation.

julia> VERSION
v"1.9.3"

julia> dump(:[(i==2) ? i * 6 : i for i = 1 : 4])
Expr
  head: Symbol comprehension
  args: Array{Any}((1,))
    1: Expr
      head: Symbol generator
      args: Array{Any}((2,))
        1: Expr
          head: Symbol if
          args: Array{Any}((3,))
            1: Expr
              head: Symbol call
              args: Array{Any}((3,))
                1: Symbol ==
                2: Symbol i
                3: Int64 2
            2: Expr
              head: Symbol call
              args: Array{Any}((3,))
                1: Symbol *
                2: Symbol i
                3: Int64 6
            3: Symbol i
        2: Expr
          head: Symbol =
          args: Array{Any}((2,))
            1: Symbol i
            2: Expr
              head: Symbol call
              args: Array{Any}((3,))
                1: Symbol :
                2: Int64 1
                3: Int64 4

julia>

@btmit
Copy link
Author

btmit commented Mar 19, 2024

Thanks for the pointer!

Do you remember what the last version that worked was? The output appears the same as far back as 1.6.7 1.0.5.

@StephenVavasis
Copy link
Owner

It appears that I last worked on the code around 2015, when Julia was at about v. 0.5.

@btmit
Copy link
Author

btmit commented Mar 20, 2024

Okay, it looks to me that 0.5 broke this by making a comprehension wrap a generator.

0.4.7 (2016)

I believe this is the last one that would have worked

julia> dump(:[(i==2) ? i * 6 : i for i = 1 : 4])
Expr
head: Symbol comprehension
args: Array(Any,(2,))
1: Expr
head: Symbol if
args: Array(Any,(3,))
1: Expr
head: Symbol comparison
args: Array(Any,(3,))
typ: Any
2: Expr
head: Symbol call
args: Array(Any,(3,))
typ: Any
3: Symbol i
typ: Any
2: Expr
head: Symbol =
args: Array(Any,(2,))
1: Symbol i
2: Expr
head: Symbol :
args: Array(Any,(2,))
typ: Any
typ: Any
typ: Any

0.5.2 (2017)

Added generator (this is what most likely broke it)

julia> dump(:[(i==2) ? i * 6 : i for i = 1 : 4])
Expr
head: Symbol comprehension
args: Array{Any}((1,))
1: Expr
head: Symbol generator
args: Array{Any}((2,))
1: Expr
head: Symbol if
args: Array{Any}((3,))
1: Expr
head: Symbol call
args: Array{Any}((3,))
1: Symbol ==
2: Symbol i
3: Int64 2
typ: Any
2: Expr
head: Symbol call
args: Array{Any}((3,))
1: Symbol *
2: Symbol i
3: Int64 6
typ: Any
3: Symbol i
typ: Any
2: Expr
head: Symbol =
args: Array{Any}((2,))
1: Symbol i
2: Expr
head: Symbol :
args: Array{Any}((2,))
1: Int64 1
2: Int64 4
typ: Any
typ: Any
typ: Any
typ: Any

0.7.0 (2018)

Changed a little bit again (see very end)

julia> dump(:[(i==2) ? i * 6 : i for i = 1 : 4])
Expr
head: Symbol comprehension
args: Array{Any}((1,))
1: Expr
head: Symbol generator
args: Array{Any}((2,))
1: Expr
head: Symbol if
args: Array{Any}((3,))
1: Expr
head: Symbol call
args: Array{Any}((3,))
1: Symbol ==
2: Symbol i
3: Int64 2
2: Expr
head: Symbol call
args: Array{Any}((3,))
1: Symbol *
2: Symbol i
3: Int64 6
3: Symbol i
2: Expr
head: Symbol =
args: Array{Any}((2,))
1: Symbol i
2: Expr
head: Symbol call
args: Array{Any}((3,))
1: Symbol :
2: Int64 1
3: Int64 4

1.10.2 (2024)

Same as 0.7.0, so it's been stable for six years.

It looks like this can be fixed by replacing expr.args with expr.args[1].args everywhere in the @tuplegen macro definition.

@StephenVavasis
Copy link
Owner

Glad you figured this out! If you want to submit a PR to fix it, I'll be happy to approve.

@btmit
Copy link
Author

btmit commented Mar 21, 2024

Unfortunately this is the best I can do.

macro tuplegen(expr)
    if expr.head != :comprehension || length(expr.args[1].args) != 2 ||
        expr.args[1].args[2].head != :(=) ||
        typeof(expr.args[1].args[2].args[1]) != Symbol
        error("Expression following tuplegen macro must be a comprehension as described in the documentation")
    end
    varname = expr.args[1].args[2].args[1]
    ret = Expr(:tuple)
    for k in Core.eval(__module__, expr.args[1].args[2].args[2])
        e2 = copy_and_substitute_tree(expr.args[1].args[1], varname, k, __module__)
        push!(ret.args,e2)
    end
    esc(ret)
end

@StephenVavasis
Copy link
Owner

Does this seem to fix the problem? Why did you use the word "unfortunately" in your previous post?

@btmit
Copy link
Author

btmit commented Mar 26, 2024

"Unfortuantely" was simply in regard to having to give you a code snippet instead of a proper PR.

Also, as I learn more about Julia marcos, I believe the following would work as well.

macro tuplegen(expr) 
    Expr(:tuple, :($expr...))
end

julia> @tuplegen [(i==2) ? i * 6 : i for i = 1 : 4]
(1, 12, 3, 4)

@StephenVavasis
Copy link
Owner

I think the macro in your previous posting will simply emit tuple([(i==2) ? i*6 : i for i=1:4]). In other words, it won't unroll, and it may not necessarily skip the intermediate step of allocating a temporary array on the heap.

@btmit
Copy link
Author

btmit commented Mar 28, 2024

You're right, that allocates. This doesn't seem to:

macro tuplegen(expr)
    Expr(:tuple, eval(expr)...)
end

@StephenVavasis
Copy link
Owner

StephenVavasis commented Mar 29, 2024

The code in your previous posting evaluates the code entirely at macro-expansion time. This will work only if the tuple does not involve any program variables other than the loop index. For example, the macro in your previous post would not work in the following example:

a = pop!(somevector)
@tuplegen [a * i for i = 1 : 5]

because the value of a is not known at macro-expansion time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants