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

Cannot specify type of list comprehension #1343

Closed
LilithHafner opened this issue Dec 25, 2022 · 5 comments
Closed

Cannot specify type of list comprehension #1343

LilithHafner opened this issue Dec 25, 2022 · 5 comments

Comments

@LilithHafner
Copy link
Contributor

Zygote can differentiate list comprehensions with automatically generated types ([i+1 for i in 1:10]), but not for explicitly specified types (Int[i+1 for i in 1:10] or Float64[i+1 for i in 1:10]).

MWE

julia> gradient(1.0) do x
           sum([x for _ in 1:10])
       end
(10.0,)

julia> gradient(1.0) do x
           sum(Float64[x for _ in 1:10])
       end
ERROR: Mutating arrays is not supported -- called setindex!(Vector{Float64}, ...)
This error occurs when you ask Zygote to differentiate operations that change
the elements of arrays in place (e.g. setting values with x .= ...)

Possible fixes:
- avoid mutating operations (preferred)
- or read the documentation and solutions for this error
  https://fluxml.ai/Zygote.jl/latest/limitations

Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] _throw_mutation_error(f::Function, args::Vector{Float64})
   @ Zygote ~/.julia/packages/Zygote/SmJK6/src/lib/array.jl:86
 [3] (::Zygote.var"#389#390"{Vector{Float64}})(#unused#::Nothing)
   @ Zygote ~/.julia/packages/Zygote/SmJK6/src/lib/array.jl:98
 [4] (::Zygote.var"#2509#back#391"{Zygote.var"#389#390"{Vector{Float64}}})(Δ::Nothing)
   @ Zygote ~/.julia/packages/ZygoteRules/AIbCs/src/adjoint.jl:67
 [5] Pullback
   @ ./REPL[49]:2 [inlined]
 [6] (::typeof((#47)))(Δ::Float64)
   @ Zygote ~/.julia/packages/Zygote/SmJK6/src/compiler/interface2.jl:0
 [7] (::Zygote.var"#60#61"{typeof((#47))})(Δ::Float64)
   @ Zygote ~/.julia/packages/Zygote/SmJK6/src/compiler/interface.jl:45
 [8] gradient(f::Function, args::Float64)
   @ Zygote ~/.julia/packages/Zygote/SmJK6/src/compiler/interface.jl:97
 [9] top-level scope
   @ REPL[49]:1

julia> VERSION
v"1.9.0-alpha1"

(jl_NHms0y) pkg> status
Status `/private/var/folders/hc/fn82kz1j5vl8w7lwd4l079y80000gn/T/jl_NHms0y/Project.toml`
  [e88e6eb3] Zygote v0.6.51
@ToucheSir
Copy link
Member

julia> Meta.@lower Int[i+1 for i in 1:10]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope`
1 ─ %1  = 1:10
│   %2  = Base.IteratorSize(%1)
│   %3  = %2 isa Base.SizeUnknown
│   %4  = Base._array_for(Int, %1, %2)
│   %5  = Base.LinearIndices(%4)
│         @_2 = Base.first(%5)
│         #s1 = Base.iterate(%1)
│   %8  = #s1 === nothing
│   %9  = Base.not_int(%8)
└──       goto #7 if not %9
2 ┄ %11 = #s1
│         i = Core.getfield(%11, 1)
│   %13 = Core.getfield(%11, 2)
│   %14 = i + 1
│         nothing
└──       goto #4 if not %3
3 ─       Base.push!(%4, %14)
└──       goto #5
4 ─       Base.setindex!(%4, %14, @_2)
5 ┄       nothing
│         @_2 = Base.add_int(@_2, 1)
│         #s1 = Base.iterate(%1, %13)
│   %23 = #s1 === nothing
│   %24 = Base.not_int(%23)
└──       goto #7 if not %24
6 ─       goto #2
7 ┄       return %4
))))

Not much we can do when the compiler unconditionally lowers to code using mutation, unfortunately.

@LilithHafner
Copy link
Contributor Author

#75 would be a great fix :)

@ToucheSir
Copy link
Member

ToucheSir commented Dec 25, 2022

There's also an infinitesimally small chance it'll happen as things stand with Zygote development. The only reason I haven't added the wontfix label here is in the off change the lowering for that syntax might change in some future Julia version. For now, we can use GitHub's suggested verbiage and say this is "not planned".

@ToucheSir ToucheSir closed this as not planned Won't fix, can't repro, duplicate, stale Dec 25, 2022
@LilithHafner
Copy link
Contributor Author

LilithHafner commented Dec 25, 2022

Is the plan to not add support for mutation? (duplicate of #75 (comment))

@ToucheSir
Copy link
Member

ToucheSir commented Dec 25, 2022

There are no plans until someone comes up with a proposal/PR that overcomes everything mentioned in #1228 (comment). Thus far this has not happened, and so nothing is planned.

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

No branches or pull requests

2 participants