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

Range-indexing FixedSizeArrays must/could return other FixedSizeArrays or "SubFixedSizeArray"s #17

Open
cdsousa opened this issue Aug 28, 2015 · 7 comments

Comments

@cdsousa
Copy link
Contributor

cdsousa commented Aug 28, 2015

No description provided.

@schlady
Copy link
Contributor

schlady commented Dec 17, 2015

+1
will work on this in the next days

@c42f
Copy link
Collaborator

c42f commented Feb 8, 2016

I just ran into this as well. We should definitely return a fixed size array for slices, at the moment a tuple comes out:

julia> Vec(1,2,3)[1:2]
(1,2)

@cdsousa
Copy link
Contributor Author

cdsousa commented Feb 8, 2016

Then, there is also the question of type stability of such indexing.

A workaround I've been using (told by @SimonDanisch ) is using Val-encapsulated indexes: Vec(1,2,3)[Val{1:2}] and generated functions:

@generated function Base.getindex{R}(v::Vec, i::Type{Val{R}})
    :(Vec(tuple($([:(v._[$i]) for i in R]...))))
end

@generated function Base.getindex{Ri, Rj}(m::Mat, ::Type{Val{Ri}}, ::Type{Val{Rj}})
    if isa(Rj, Integer)
        :(Vec(tuple($([:(m._[$Rj][$i]) for i in Ri]...))))
    else
        :(Mat(tuple($([:(tuple($([:(m._[$j][$i]) for i in Ri]...))) for j in Rj]...))))
    end
end

@cdsousa
Copy link
Contributor Author

cdsousa commented Feb 8, 2016

But for a first approach we may have to forget about type stability, and just improve current getindex methods of https://github.com/SimonDanisch/FixedSizeArrays.jl/blob/master/src/indexing.jl#L1-L6.

@cdsousa
Copy link
Contributor Author

cdsousa commented Feb 8, 2016

BTW, this Vec(Vec(1,2,3)[1:2]) already works. It doesn't work well for Mats though.

@SimonDanisch
Copy link
Owner

I could give it another go now that we have a "better" similar function ;)
But type instability will remain, as long as we don't use something like Val or get const propagation.
Although, from vec[(1,2,3)] the type should be inferrable just fine...
We could think about introducing some fixed size range type :D

@c42f
Copy link
Collaborator

c42f commented Feb 9, 2016

Good point about the type problems. The tuple approach seems like the best thing we can do at the moment IMHO, though for anything larger than 3 or 4-vectors, the Val approach would make some sense regardless of the ugly syntax.

Constant propagation has been discussed a little at JuliaLang/julia#5560 - I imagine even one pass of this would solve a whole raft of what are currently tricky design problems. Unfortunately doesn't look like it's coming any time soon!

@c42f c42f mentioned this issue Mar 10, 2016
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