MaxPool not working on latest master branch

[roberthoenig]

using Flux

model = Chain(
    Conv((2,2), 1=>16, relu),
    MaxPool((2,2)),
    x -> reshape(x, :, size(x, 4)),
    Dense(144, 10),
    softmax);

X = rand(8,8,1,4);

loss(x) = begin
    Flux.crossentropy(model(x), rand(10,4))
end
opt = Flux.RMSProp(Flux.params(model));

Flux.train!(loss, [(X,)], opt)

The above code works fine in [587475ba] Flux v0.6.9, but it crashes on the latest master (commit 4cba46c) with the following error message (pruned):

MethodError: no method matching ∇maxpool(::Array{Float64,4}, ::Array{Float32,4}, ::Array{Float32,4}, ::Tuple{Int64,Int64}; pad=(0, 0), stride=(2, 2))
Closest candidates are:
∇maxpool(::A<:AbstractArray, !Matched::A<:AbstractArray, !Matched::A<:AbstractArray, ::Any; pad, stride) where A<:AbstractArray at /home/robert/.julia/packages/NNlib/0EAe7/src/conv.jl:142
Stacktrace:
[1] (::getfield(Flux.Tracker, Symbol("##457#458")){Base.Iterators.Pairs{Symbol,Tuple{Int64,Int64},Tuple{Symbol,Symbol},NamedTuple{(:pad, :stride),Tuple{Tuple{Int64,Int64},Tuple{Int64,Int64}}}},TrackedArray{Float32,4,Array{Float32,4}},Tuple{Int64,Int64},Array{Float32,4}})(::Array{Float64,4}) at /home/robert/.julia/dev/Flux/src/tracker/lib/array.jl:393

[Roger-luo]

I guess

X = rand(Float32, 8, 8, 1, 4)

might fix your error? This is because the function requires input with same array type:

https://github.com/FluxML/NNlib.jl/blob/20aa7005c2ad2e511b4332a8af7920a2bac7a067/src/conv.jl#L162

BTW, please use a code block for error msg instead of quote, I'm the issuer from issue #457 ...

[eamartin]

I think the surprise here comes from differing default Float types. My understanding is that things in Julia default to Float64. Something in this chain must default to Float32 and mess things up. I'll investigate.

[Roger-luo]

The default type in flux is float32, this is because most GPU device support float 32 only so most framework like pytorch use it as default

[Roger-luo]

I think use float32 as default is more intuitive when using Julia as well

[eamartin]

I've only been using Flux for a day, but I assumed default float type as Float64 due to following behavior

julia> using Flux

julia> typeof(Dense(2, 1))
Dense{typeof(identity),TrackedArray{…,Array{Float64,2}},TrackedArray{…,Array{Float64,1}}}

In what sense is Float32 the default?

[eamartin]

In fact, I don't think there's currently a clean way to initialize a Dense layer to use Float32 instead of Float64, as glorot_uniform or glorot_normal don't take an argument for dtype.
This seems like a separate issue/work-item: better support for picking dtypes.

I agree that default dtype should be Float32. Most users will use the default, and going with Float64 will make Flux slower than frameworks that use Float32 by default (definitely TensorFlow, and @Roger-luo says also PyTorch)

[Roger-luo]

@eamartin I had a PR to initialize Dense with given type. Haven't finished it yet

[Roger-luo]

I'm currently using a self defined Linear like torch with my custom fork…

[eamartin]

@Roger-luo was this just for Dense or for all layers and initializers in Flux?

I don't want to duplicate work you're already doing. But if desired, I could create a PR to add more natural dtype support.

[Roger-luo]

I just added several type parameters to Dense, and try to make it follow the Array constructor style, and I also have a Linear (like torch), that's all I have now, don't have time to change other layers now.

[Roger-luo]

Sorry, I was going to say make float64 as default. Because most Julia packages use float64 including stdlib, this follows the CPU convention, which is more intuitive in Julia. In Python, it is because each framework have their own data system, you don't need to worry about the interaction between each package. And we can always throw a warning if the GPU device does not actually support float64

[eamartin]

@Roger-luo I opened a PR with configurable dtypes #513
This PR (applied to master at a32c8a2) does not fix this issue (including if X = rand(...) is switched to Float32 as well as the target rand(10, 4).

I grepped through NNlib and I don't see anywhere where it converts things to Float64. My guess is that mixed precision starts from the optimizers still having Float64 (hyper-)parameters.

[Roger-luo]

Yeah, I didn't asked about this concern about optimizer in slack… no one replied me…

[DhairyaLGandhi]

Also, converting both X and the rand in the loss to Float32 works for me.

[AkashGanesan]

Is this issue fixed? I don't get this error anymore.

[DhairyaLGandhi]

It should just work with NNlib v0.6

[AkashGanesan]

Is there a reference to the NNlib issue? Thanks!