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

StaticArrays seems to apply promotion well for Base number types but not dual numbers #497

Closed
jrevels opened this issue Sep 17, 2018 · 3 comments

Comments

@jrevels
Copy link

jrevels commented Sep 17, 2018

julia> using ForwardDiff, StaticArrays

julia> x = ForwardDiff.Dual(1.0)
Dual{Nothing}(1.0)

julia> SMatrix{2,2}(x, 2, 3.0, 4.0)
2×2 SArray{Tuple{2,2},Real,2,4}:
 Dual{Nothing}(1.0)  3.0
               2     4.0

julia> promote(x, 2, 3.0, 4.0)
(Dual{Nothing}(1.0), Dual{Nothing}(2.0), Dual{Nothing}(3.0), Dual{Nothing}(4.0))

julia> SMatrix{2,2}(Int32(1), 2, 3.0, 4.0)
2×2 SArray{Tuple{2,2},Float64,2,4}:
 1.0  3.0
 2.0  4.0
@chethega
Copy link
Contributor

chethega commented Sep 18, 2018

Ok, so this bug is super weird and I have the nagging feeling that it is upstream (in inference? Include/compile-order dependent?).

julia> using ForwardDiff, StaticArrays

julia> x = ForwardDiff.Dual(1.0); tup=(x, 1);

julia> SMatrix{2,1}(tup)
2×1 SArray{Tuple{2,1},Real,2,2}:
 Dual{Nothing}(1.0)
               1   

julia> @which SMatrix{2,1}(tup)
(::Type{SArray{Tuple{S1,S2},T,2,L} where L where T})(x::Tuple{Vararg{Any,L}}) where {S1, S2, L} in StaticArrays at ~/.julia/dev/StaticArrays/src/SMatrix.jl:33

Let's look into the code and copy-paste it into the REPL

julia> import StaticArrays.SMatrix, StaticArrays.promote_tuple_eltype

julia> @generated function (::Type{SMatrix{S1,S2}})(x::NTuple{L,Any}) where {S1,S2,L}
           T = promote_tuple_eltype(x)

           return quote
               $(Expr(:meta, :inline))
               SMatrix{S1, S2, $T, L}(x)
           end
       end

julia> @which SMatrix{2,1}(tup)
(::Type{SArray{Tuple{S1,S2},T,2,L} where L where T})(x::Tuple{Vararg{Any,L}}) where {S1, S2, L} in Main at REPL[6]:2

julia> SMatrix{2,1}(tup)
2×1 SArray{Tuple{2,1},ForwardDiff.Dual{Nothing,Float64,0},2,2}:
 Dual{Nothing}(1.0)
 Dual{Nothing}(1.0)

@andyferris
Copy link
Member

Yeah, no, that's definitely our fault!

We should not be calling promote_tuple_eltype from the generator. It needs to be put into the generated code. There will be no overhead for this.

(For some history, a lot of this package was written before the world-age-consistency system was completed, and particulary before it was understood (by myself). So there's some warts like this left and we should really audit the code for stuff like this, since it behaves so inconsistently)

@c42f
Copy link
Member

c42f commented Feb 1, 2019

I think was fixed a while back in #503

@c42f c42f closed this as completed Feb 1, 2019
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

4 participants