Open bvdmitri opened 3 months ago
I think the reason ForwardDiff is confused because this ArrayPartition declares itself to have Float64 elements, but in fact returns an Int sometimes:
julia> ap = ArrayPartition([ 0.0 ], [ 1 ])
([0.0], [1])
julia> size(ap), axes(ap), eltype(ap), supertype(typeof(ap))
((2,), (Base.OneTo(2),), Float64, AbstractVector{Float64})
julia> ap[1] # no surprise
0.0
julia> ap[2] # very surprising
1
julia> ap[1:2] # here you get the expected eltype
2-element Vector{Float64}:
0.0
1.0
julia> ap[2,1:1]
1-element Vector{Float64}:
1.0
The usual way to encode that elements of a vector have different types is to have an abstract eltype, which it seems ForwardDiff is able to handle.
(Note that the other example above constructs a Vector{Float64}
, promoting to 1.0
when making the array.)
julia> x64 = [ 0.0, 1 ] # this promotes on construction
2-element Vector{Float64}:
0.0
1.0
julia> ForwardDiff.gradient(f, x64) # as in question
2-element Vector{Float64}:
1.0
1.0
julia> xabs = Real[ 0.0, 1 ] # abstract eltype, could also use xabs = Union{Float64, Int}[ 0.0, 1 ]
2-element Vector{Real}:
0.0
1
julia> ForwardDiff.gradient(f, xabs) # also OK, ForwardDiff not confused
2-element Vector{Float64}:
1.0
1.0
Fixing ArrayPartition to declare its eltype accurately would be the obvious fix here, and would probably avoid many other weird edge cases. (Or else fixing its getindex
definition to convert to the declared eltype.) Although I'm sure there's going to be some reason that consistency is inconvenient for something.
It's possible that ForwardDiff could be made more robust to misleading signals. For instance making the ForwardDiff.Dual
constructor called above promote its first argument might work here?
Opened an issue in RecursiveArrayTools
as well, though, I have a feeling that this behaviour might be by design.
For instance making the ForwardDiff.Dual constructor called above promote its first argument might work here?
For me that would be an obvious fix, that shouldn't break anything, right?
I have a feeling that this behaviour might be by design.
That just seems broken though. If fixing RecursiveArrayTools also fixes this then I don't think anything should be done here.
ArrayPartition
is a useful structure to concatenate arrays of different types. The type is defined in SciML/RecursiveArrayTools.jlArrayPartition
s are also used in many places inSciML
ecosystem, but also in other places likeManopt.jl
. It appears, though, that ifArrayPartition
references two containers, one ofeltype
isFloat64
and another one isInt64
, thegradient
fromForwardDiff
fails.MWE is: