Skip to content

Error during SArray initialization with array comprehension containing two SVector iterates #76

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

Closed
wsshin opened this issue Nov 10, 2016 · 6 comments

Comments

@wsshin
Copy link
Contributor

wsshin commented Nov 10, 2016

Consider two SVector objects with 2 and 3 entries:

julia> vec2 = @SVector [1,2]
2-element StaticArrays.SVector{2,Int64}:
 1
 2

julia> vec3 = @SVector [1,2,3]
3-element StaticArrays.SVector{3,Int64}:
 1
 2
 3

I can use one of these two SVector objects as iterates in array comprehension to initialize SArray. For example,

julia> @SArray [1 for x = vec3, y = vec3]
3×3 StaticArrays.SArray{(3,3),Int64,2,9}:
 1  1  1
 1  1  1
 1  1  1

However, as soon as I mix two SVector objects, an error is generated:

julia> @SArray [1 for x = vec2, y = vec3]
ERROR: The size of type `StaticArrays.SVector{S,Int64}` is not known.

If you were trying to construct (or `convert` to) a `StaticArray` you
may need to add the size explicitly as a type parameter so its size is
inferrable to the Julia compiler (or performance would be terrible). For
example, you might try

    m = zeros(3,3)
    SMatrix(m)      # this error
    SMatrix{3,3}(m) # correct - size is inferrable

Is this an expected behavior?

Here is the versioninfo() output:

julia> versioninfo()
Julia Version 0.5.1-pre+4
Commit 887ad05* (2016-10-28 19:39 UTC)
Platform Info:
  System: Darwin (x86_64-apple-darwin16.0.0)
  CPU: Intel(R) Core(TM)2 Duo CPU     T9900  @ 3.06GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Penryn)
  LAPACK: libopenblas64_
  LIBM: libopenlibm
  LLVM: libLLVM-3.7.1 (ORCJIT, penryn)
@andyferris
Copy link
Member

Thanks @wsshin for the report. Comprehensions (and generators) certainly are only partially supported at the moment, and we want to support them better in the future, but it is rather odd that one works and the other doesn't.

@c42f
Copy link
Member

c42f commented Nov 12, 2016

This seems to be due to a bizarre interaction between type inference with size:

julia> using StaticArrays

julia> vec2 = @SVector([1,2]); vec3 = @SVector([1,2,3])
3-element StaticArrays.SVector{3,Int64}:
 1
 2
 3

julia> [eval(ex) for ex in [:vec2, :vec3]]
ERROR: The size of type `StaticArrays.SVector{S,Int64}` is not known.

Type inference has inferred SVector{S,Int} for the comprehension, and for some reason you cannot construct an array from these:

julia> typealias SVecInt{S} SVector{S,Int}
StaticArrays.SVector{S,Int64}

julia> sv = SVecInt[SVector(1,2)];
ERROR: The size of type `StaticArrays.SVector{S,Int64}` is not known.

@c42f
Copy link
Member

c42f commented Nov 12, 2016

This is super easy to workaround in the macro, but the root cause is confusing. Typed comprehension lowers to a weird getindex call -

getindex(SVecInt, SVector(1,2))

Which in turn does this:

julia> a = Array{SVecInt,1}(1)
1-element Array{StaticArrays.SVector{S,Int64},1}:
 #undef

julia> a[1] = SVector(1,2)
ERROR: The size of type `StaticArrays.SVector{S,Int64}` is not known.

@c42f
Copy link
Member

c42f commented Nov 12, 2016

Ultimately, this boils down to convert being called in Base.setindex!:

convert(SVecInt, SVector(1,2))

As it turns out, running with --inline=no gives a much less confusing stacktrace:

julia> SVecInt[SVector(1,2)]
ERROR: The size of type `StaticArrays.SVector{S,Int64}` is not known.

If you were trying to construct (or `convert` to) a `StaticArray` you
may need to add the size explicitly as a type parameter so its size is
inferrable to the Julia compiler (or performance would be terrible). For
example, you might try

    m = zeros(3,3)
    SMatrix(m)      # this error
    SMatrix{3,3}(m) # correct - size is inferrable

 in error(::String) at ./error.jl:21
 in size(::Type{StaticArrays.SVector{S,Int64}}) at /home/chris/.julia/v0.5/StaticArrays/src/core.jl:155
 in length(::Type{StaticArrays.SVector{S,Int64}}) at /home/chris/.julia/v0.5/StaticArrays/src/abstractarray.jl:1
 in StaticArrays.SVector{S,Int64}(::Tuple{Int64,Int64}) at /home/chris/.julia/v0.5/StaticArrays/src/core.jl:46
 in convert(::Type{StaticArrays.SVector{S,Int64}}, ::StaticArrays.SVector{2,Int64}) at /home/chris/.julia/v0.5/StaticArrays/src/core.jl:69
 in setindex!(::Array{StaticArrays.SVector{S,Int64},1}, ::StaticArrays.SVector{2,Int64}, ::Int64) at ./array.jl:415
 in getindex(::Type{StaticArrays.SVector{S,Int64}}, ::StaticArrays.SVector{2,Int64}) at ./array.jl:138
 in eval(::Module, ::Any) at ./boot.jl:234

c42f added a commit that referenced this issue Nov 12, 2016
This doesn't address the root cause, but works around the issue reported
in #76.
@andyferris
Copy link
Member

By the way, @wsshin the comprehension syntax I've implemented is only meant for literals like @SVector [i for i = 1:10]. In no case will your example compile to fast code inside a function.

We definitely want to support this behaviour (the output dimension is inferrable from the static vector inputs) but it's just not implemeted yet.

@andyferris
Copy link
Member

See #26

oschulz pushed a commit to oschulz/StaticArrays.jl that referenced this issue Apr 4, 2023
Use normal functions for getindex
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