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

Moving length here #14

Closed
mcabbott opened this issue Dec 10, 2022 · 2 comments
Closed

Moving length here #14

mcabbott opened this issue Dec 10, 2022 · 2 comments

Comments

@mcabbott
Copy link

mcabbott commented Dec 10, 2022

For JuliaDiff/ForwardDiff.jl#599 to work, length(::Type{StaticArraysCore.SMatrix{3, 3, Float64, 9}}) needs to be defined here in StaticArraysCore.

The present definition is:

https://github.com/JuliaArrays/StaticArrays.jl/blob/f7b8ed21c946fef2e9ecb00d7a27bccdd2623976/src/abstractarray.jl#L1-L2

which needs

https://github.com/JuliaArrays/StaticArrays.jl/blob/28e0482ba006bfb9a4a4f19dbf8b3096b2611497/src/StaticArrays.jl#L71-L86

which depends on LinearAlgebra.

What should happen? This core package has no dependencies right now. Should it gain LinearAlgebra just for this? Otherwise it could just define length for a smaller set of types, and let the main package extend that. I think that would be sufficient for ForwardDiff, whose generated functions e.g. here https://github.com/JuliaDiff/ForwardDiff.jl/blob/c6dc2098b5201943cdb8433ee963078e6dcc7900/src/apiutils.jl#L21-L25 accept only x::StaticArray, never Diagonal(StaticVector) etc.

@mateuszbaran
Copy link
Collaborator

I think it would be the easiest if ForwardDiff just used tuple_prod instead of length like here:

@inline SArray{S,T,N}(x::Tuple) where {S<:Tuple,T,N} = SArray{S,T,N,tuple_prod(S)}(x)
.

@mcabbott
Copy link
Author

mcabbott commented Dec 12, 2022

Thanks, that indeed solves the problem. Done here: JuliaDiff/ForwardDiff.jl@f3afe13

Maybe the next one is harder. While #5 moved similar_type here, its methods are all from the main package, and these seem to be called within @generated code, leading to:

StaticArraysCore.MArray: Error During Test at /Users/me/.julia/dev/ForwardDiff/test/GradientTest.jl:87
  Got exception outside of a @test
  MethodError: no method matching similar_type(::Type{StaticArraysCore.MMatrix{3, 3, Float64, 9}}, ::Type{ForwardDiff.Dual{ForwardDiff.Tag{typeof(prod), Float64}, Float64, 9}})
  The applicable method may be too new: running in world age 33438, while current world is 33489.
  
  Closest candidates are:
    similar_type(::Type{SA}, ::Type{T}, ::StaticArraysCore.Size{S}) where {SA<:StaticArraysCore.MArray, T, S} (method too new to be called from this world context.)
     @ StaticArrays ~/.julia/packages/StaticArrays/B0HhH/src/abstractarray.jl:76
    similar_type(::Type{SA}, ::Type{T}) where {SA<:(Union{LinearAlgebra.Adjoint{T, <:Union{StaticArraysCore.StaticArray{Tuple{var"#s2"}, T, 1} where var"#s2", StaticArraysCore.StaticArray{Tuple{var"#s3", var"#s4"}, T, 2} where {var"#s3", var"#s4"}}}, LinearAlgebra.Diagonal{T, <:StaticArraysCore.StaticArray...  (method too new to be called from this world context.)
     @ StaticArrays ~/.julia/packages/StaticArrays/B0HhH/src/abstractarray.jl:37
    similar_type(::Type{A}, ::Type{T}, ::StaticArraysCore.Size{S}) where {A<:AbstractArray, T, S} (method too new to be called from this world context.)
     @ StaticArrays ~/.julia/packages/StaticArrays/B0HhH/src/abstractarray.jl:73
    ...

Edit -- oh nevermind, I think I can just move that call from the body into the quote. As was done in other places, already.

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

2 participants