Flux.jl: `activations` is broken

Created on 26 Mar 2019  Â·  4Comments  Â·  Source: FluxML/Flux.jl

In Flux v0.6.1, the last tutorial code sample in regularisation fails

using Flux
c = Chain(Dense(10,5,σ),Dense(5,2),softmax)
Flux.activations(c, rand(10))

which raises ERROR: MethodError: no method matching accumulate(::getfield(Flux, Symbol("##62#63")), ::Array{Float64,1}, ::Array{Any,1})

It seems that accumulate(op, A; dims::Integer, [init]) doesn't support multiple inputs as you wrote in activations(c::Chain, x) = accumulate((x, m) -> m(x), x, c.layers) anymore.

_Originally posted by @johnnychen94 in https://github.com/FluxML/Flux.jl/pull/174#issuecomment-414040344_

bug help wanted

Most helpful comment

Meh. That would be nice but the important thing is that this works, and your implementation is totally reasonable. So I think we can close this for now and change it if accumulate every improves.

All 4 comments

I closed my patch PR https://github.com/FluxML/Flux.jl/pull/355 because @MikeInnes at that time you fixed it by yourself https://github.com/FluxML/Flux.jl/commit/9d1d5187f349252365e73adb7a2da66caf29bfcf.

Now It's actually another bug:

Flux.activations(Chain(Dense(10,5)), rand(10))
ERROR: MethodError: no method matching similar(::Tuple{Dense{typeof(identity),TrackedArray{…,Array{Float32,2}},TrackedArray{…,Array{Float32,1}}}}, ::Type{Any})
Closest candidates are:
  similar(::Array{T,1}, ::Type) where T at array.jl:314
  similar(::Array{T,2}, ::Type) where T at array.jl:315
  similar(::Array, ::Type, ::Tuple{Vararg{Int64,N}}) where N at array.jl:317
  ...
Stacktrace:
 [1] #accumulate#585(::Nothing, ::Base.Iterators.Pairs{Symbol,Array{Float64,1},Tuple{Symbol},NamedTuple{(:init,),Tuple{Array{Float64,1}}}}, ::Function, ::Function, ::Tuple{Dense{typeof(identity),TrackedArray{…,Array{Float32,2}},TrackedArray{…,Array{Float32,1}}}}) at ./accumulate.jl:243
 [2] (::getfield(Base, Symbol("#kw##accumulate")))(::NamedTuple{(:init,),Tuple{Array{Float64,1}}}, ::typeof(accumulate), ::Function, ::Tuple{Dense{typeof(identity),TrackedArray{…,Array{Float32,2}},TrackedArray{…,Array{Float32,1}}}}) at ./none:0
 [3] activations(::Chain{Tuple{Dense{typeof(identity),TrackedArray{…,Array{Float32,2}},TrackedArray{…,Array{Float32,1}}}}}, ::Array{Float64,1}) at /home/math/jc/.julia/packages/Flux/8XpDt/src/layers/basic.jl:43
 [4] top-level scope at none:0

Here's my original patch, basically a hand-written accumulate and it works:

activations(m::Any, x, rst=[]) = push!(rst,m(x))
activations(c::Chain, x, rst=[]) = begin
    rst = activations(c[1], x, rst)
    if length(c) >= 2
        rst = activations(c[2:end], rst[end], rst)
    end
    return rst
end

reduce is surprisingly broken in Julia. It'd be nice just to have a simple reduce-over-tuples (should be a couple lines) and use that. Or figure out why else this went wrong.

A temporary patch is implemented in https://github.com/FluxML/Flux.jl/pull/710
However, this issue should be kept open unless there's a real reason to have a Flux-version reduce (e.g., fix on upstream becomes impossible)

Meh. That would be nice but the important thing is that this works, and your implementation is totally reasonable. So I think we can close this for now and change it if accumulate every improves.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

caseykneale picture caseykneale  Â·  5Comments

KristofferC picture KristofferC  Â·  4Comments

sklan picture sklan  Â·  6Comments

pylat picture pylat  Â·  3Comments

aminya picture aminya  Â·  4Comments