-
Notifications
You must be signed in to change notification settings - Fork 150
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
FixedSizeArrays --> StaticArrays #45
Comments
and then there was 3... :P. It seems that your points are not very difficult to implement. A new package seems wholly unnecessary in my view. |
Looks like normalize is slow because it's going via Base and calling |
The issue of julia> A = [[0,1], [1,2]]
2-element Array{Array{Int64,1},1}:
[0,1]
[1,2]
julia> A .+ [1,-1]
2-element Array{Array{Int64,1},1}:
[1,2]
[0,1] In this case, julia> B = [[0,1], [1,2], [1,0]]
3-element Array{Array{Int64,1},1}:
[0,1]
[1,2]
[1,0]
julia> map(x->x+[1,-1], B)
3-element Array{Array{Int64,1},1}:
[1,0]
[2,1]
[2,-1]
julia> B .+ [1,-1]
ERROR: DimensionMismatch("arrays could not be broadcast to a common size") I don't know whether there's a way to resolve this when |
Hmm, I wonder if a recursive extension to function Base.broadcast{A<:AbstractArray,T}(f, a::Array{A}, b::Array{T})
c = similar(a)
for i=eachindex(a)
c[i] = broadcast(f, a[i], b)
end
c
end
julia> B = [[0,1], [1,2], [1,0]]
3-element Array{Array{Int64,1},1}:
[0,1]
[1,2]
[1,0]
julia> B .+ [-1,1]
3-element Array{Array{Int64,1},1}:
[-1,2]
[0,3]
[0,1] I can't tell right now whether this is just a terrible idea, or it actually makes sense! |
FWIW julia> A = [[0,1], [1,2]]
2-element Array{Array{Int64,1},1}:
[0,1]
[1,2]
julia> A .+ [[1,-1]]
2-element Array{Array{Int64,1},1}:
[1,0]
[2,1] |
Yes, this is true. This is a real bummer, but I'd be reluctant to diverge from What we should do is see how we can improve the same thing in (Also note that if you have
Unfortunately then you might get people complaining they want the opposite behavior. We can still write an RFC over at JuliaLang to see what people feel.
Good solution @pabloferz. The constructors make it a bit painful to construct
Not exactly natural but it is a start! |
Yes, so true @SimonDanisch!! But my plan was to iteratively rewrite this package. There will be some API churn but that is what semver and
In any case, I think we should definitely try to pick up as much as possible from FixedSizeArrays. Please continue to report regressions as you see them and we will continue to address them (like
4 actually (ImmutableArrays was earlier). :) |
Yes we need to address this. It deserves its own issue, I think. One major decision is whether to restore the Like |
Ok, so I can't think of a reason that making Nice workaround @pabloferz. It's a pity to allocate a wrapper just to unwrap it again, but in a lot of situations the overhead is unlikely to be noticeable, eg vs = [rand(SVector{3,Float64}) for _=1:1000];
ws = vs .+ [SVector(10,0,0)]; Of course there's no need to allocate if SVector is used as the wrapper, but the syntax is pretty ugly. |
On 0.6 you can do julia> A = [SVector(1,2,3), SVector(2,3,4)]
2-element Array{StaticArrays.SVector{3,Int64},1}:
[1,2,3]
[2,3,4]
julia> (+).(A, (SVector(1,0,-1),))
2-element Array{StaticArrays.SVector{3,Int64},1}:
[2,2,2]
[3,3,3] and avoid the extra allocation (hopefully you'll be able to wrap things with The later calls the generic EDIT: I'm not sure, but I believe the last issue could also be solved during 0.6 cycle. |
Oh, great, @pabloferz I forgot about that. The issue is that |
So what are some good v0.5 workarounds? This end result is what we want to compute:
Maybe a macro for doing an elementwise operation would make this relatively painless? Such as EDIT: this can now be a function, |
Another idea is a |
I'd probably go for the scalar wrapper. Naming is tricky. |
How about |
Are we actually sure, that the Base behavior of |
I'm open to brainstorming. However keep in mind that I've been meaning to add a |
Hm, it is indeed. I guess the only reasonable extension would be to allow for: broadcast(op, ::AbstractArray{T}, ::T) To be special cased. But I'm not sure how justifiable that is. Regarding, |
@andyferris I'm not convinced by @SimonDanisch - the function |
No, I'm quite happy to be persuaded that for |
I actually got the name wrong: >julia repeated([1])
Base.Repeated{Array{Int64,1}}([1]) |
Ah right. This is almost the right kind of thing, but |
EDIT: or |
Fair point, I'd forgotten about I could go with that... immutable Fill{T} <: AbstractArray{T,0}
val::T
end
Base.size(a::Fill) = ()
Base.getindex(a::Fill) = a.val
[SVector(1,2,3), SVector(4,5,6)] .+ Fill(SVector(-1,0,0)) |
Hmm, seems fairly fundamental, does |
Are you referring to this problem: SimonDanisch/FixedSizeArrays.jl#136 I'm just a bit worried about having a bunch of almost identical names which do slightly different things in different contexts... |
Umm no I don't think so - there are a couple of issues here beyond bounds checking. A |
jeez, yeah of course... Some broadcast implementation do use |
Not sure I like that zero-dimensionality! It kind-of makes sense, though. I was expecting something more akin to Maybe a compromise would be: immutable Fill{T, N} <: AbstractArray{T,N}
val::T
end
Fill{T}(x::T) = Fill{T,1}(x)
(::Type{Fill{T}}){T}(x) = Fill{T,1}(x)
@inline Base.getindex(a::Fill) = a.val
@inline Base.getindex(a::Fill, i::Integer) = a.val
@inline Base.getindex{T,N}(a::Fill{T,N}, Vararg{Int, N}) = a.val
|
Now that broadcast is an essential part of the Julia machinery, we should try to get a few well defined types into Base to steer the behavior of broadcast. |
Yes, this seems like an issue for the standard library. |
@andyferris yes, size is a problem if you make |
|
I think a zero-D |
Yes, a scalar in the array algebra sense, but not the linear algebra sense (for that you'd need |
Here is an example that I believe should work for 0.5 (unless I'm missing something) immutable SWrap{T} <: AbstractArray
val::T
end
Base.getindex(s::SWrap, n) = s.val
Base.size(s::SWrap) = (1,)
A = [SVector(1,2,3), SVector(2,3,4)]
A .+ SWrap(SVector(1,0,-1))
(+).(A, Wrap(SVector(1,0,-1))) # Here equivalent to the previous one |
I found a surprisingly relevant comment at JuliaLang/julia#18766 (comment) by @StefanKarpinski
|
See also JuliaLang/julia#18379 |
Thanks @TotalVerb. I almost see that a zero-dimensional static arrary is the (Unfortunately |
I have a |
@SimonDanisch I ticked off item 3 from your list based on #50. I don't think we'll be able to do much better. It's merged now but if you have any feedback before tagging I'll wait a bit. |
Finally, to refocus this issue, can you elaborate again what is going wrong the constructors for you @SimonDanisch? EDIT: there was a constructor-affecting bug but that is fixed now - was it just this? |
bump |
Sorry, I need to prioritize tagging GLVisualize right now! I will have some time to elaborate afterwards! :) |
No worries. :) |
TODO here: fix the |
Let's close this one. |
The transition so far has been more painful then expected :(
I thought I should start a list with problems:
op(::Array{SVector}, ::SVector)
, e.g.a.+b
or(+).(a,b)
I repeat my concern from #11 :
Types inheriting from
StaticVector
don't inherit all constructors. Having a consistent set of constructors for different fixed arrays was an important goal for usability and because its not that trivial to implement them all correctly. Are there any plans to offer this? I must admit I got myself into quite a mess with that (the constructor code in FixedSizeArrays is probably the ugliest code I've written so far)... But maybe we could offer a macro that one could execute for a custom type?I feel a bit like writing a new package targeting 0.6 (after it got a few more features we need) with all the goodies from
FixedSizeArrays
andStaticArrays
:DThe text was updated successfully, but these errors were encountered: