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

copy(::TrackedArray) doesn't work #416

Closed
tpapp opened this issue Oct 2, 2018 · 7 comments
Closed

copy(::TrackedArray) doesn't work #416

tpapp opened this issue Oct 2, 2018 · 7 comments

Comments

@tpapp
Copy link

tpapp commented Oct 2, 2018

MWE:

using Flux.Tracker
x = ones(5)
tx = TrackedArray(x)
copy(tx)

gives the error

ERROR: MethodError: no method matching Float64(::Flux.Tracker.TrackedReal{Float64})
Closest candidates are:
  Float64(::Real, ::RoundingMode) where T<:AbstractFloat at rounding.jl:185
  Float64(::T<:Number) where T<:Number at boot.jl:722
  Float64(::Int8) at float.jl:60
  ...
Stacktrace:
 [1] convert(::Type{Float64}, ::Flux.Tracker.TrackedReal{Float64}) at ./number.jl:7
 [2] setindex!(::Array{Float64,1}, ::Flux.Tracker.TrackedReal{Float64}, ::Int64) at ./array.jl:769
 [3] setindex! at ./multidimensional.jl:414 [inlined]
 [4] copyto!(::Array{Float64,1}, ::TrackedArray{…,Array{Float64,1}}) at ./multidimensional.jl:822
 [5] copymutable(::TrackedArray{…,Array{Float64,1}}) at ./abstractarray.jl:823
 [6] copy(::TrackedArray{…,Array{Float64,1}}) at ./abstractarray.jl:773
 [7] top-level scope at none:0

on

(v1.1) pkg> status
  [587475ba] Flux v0.6.7+ #master (https://github.com/FluxML/Flux.jl.git)
julia> VERSION
v"1.1.0-DEV.361"
@MikeInnes
Copy link
Member

We just need to add a method to catch this. Since tracked arrays are immutable anyway it can just be a no-op.

@tpapp
Copy link
Author

tpapp commented Oct 2, 2018

What should similar do? Return a Array{Flux.Tracker.TrackedReal{Float64}}?

@MikeInnes
Copy link
Member

Similar should always return the equivalent un-tracked object; there's no reason to include tracking for that since there are no gradients anyway.

@tpapp
Copy link
Author

tpapp commented Oct 2, 2018

I am not sure, since the Interfaces documentation says that similar should

Return a mutable array with the same shape and element type

but in

x = TrackedArray(ones(5))
y = similar(x)
eltype(x) == eltype(y) # => false

which is causing the above code to fail.

@johnnychen94
Copy link
Contributor

johnnychen94 commented Oct 7, 2018

@tpapp we could change Base.eltype(x::Type{<:TrackedArray{T}}) where T <: Real = TrackedReal{T} in

Base.eltype(x::Type{<:TrackedArray{T}}) where T <: Real = TrackedReal{T}
to Base.eltype(x::TrackedArray)= eltype(data(x)) to solve this

@tpapp
Copy link
Author

tpapp commented Dec 23, 2018

@johnnychen94: I am not sure this is the right solution. For a bit of context, consider a function where I want to preallocate an array for results, eg

function unitvec(xs::AbstractVector{T}) where {T <: Real}
    S = typeof(one(T))         # takes care of T ≡ Int64, etc
    n = length(xs)
    y = Vector{S}(undef, n + 1)
    r = one(S)
    for (i, x) in enumerate(xs)
        z = tanh(x)
        y[i] = z * r
        r *= 1 - abs2(z)
    end
    y[end] = r
    y
end

which works fine for all kinds of arrays, except

julia> xs = Flux.param(ones(3))
Tracked 3-element Array{Float64,1}:
 1.0
 1.0
 1.0

julia> unitvec(xs)
S = Float64
ERROR: MethodError: no method matching Float64(::Flux.Tracker.TrackedReal{Float64})
Closest candidates are:
  Float64(::Real, ::RoundingMode) where T<:AbstractFloat at rounding.jl:194
  Float64(::T<:Number) where T<:Number at boot.jl:741
  Float64(::Int8) at float.jl:60
  ...
Stacktrace:
 [1] convert(::Type{Float64}, ::Flux.Tracker.TrackedReal{Float64}) at ./number.jl:7
 [2] setindex!(::Array{Float64,1}, ::Flux.Tracker.TrackedReal{Float64}, ::Int64) at ./array.jl:766
 [3] unitvec(::TrackedArray{…,Array{Float64,1}}) at ./REPL[88]:9
 [4] top-level scope at none:0

I would find it more consistent if the parameter T of a TrackedArray was the same as its eltype, and both were a TrackedReal.

@MikeInnes
Copy link
Member

I've tweaked the definition of similar in
bf0b5c5 so that it can be used for this kind of thing.

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

3 participants