Core type rewrite #144

c42f · 2016-07-11T04:36:44Z

~~WIP! Do not merge!~~

Upstream 0.5 changes seem to have broken FixedArray in a pretty fundamental way ( #137 #138 JuliaLang/julia#17247 )

I started trying to address the complete breakage in 0.5 by rethinking how the core type inheritance is arranged. There's a lot of things commented out and broken here which will need to be cleaned up later, but I'm throwing this up here for discussion.

The basic idea is that only low-dimensional fixed arrays are much use in practice since there's already a fairly strong assumption about stack allocation and loop unrolling in the existing code which only makes sense for a few elements. Given this assumption, it's probably ok to have an upper bound on the dimensionality:

abstract FixedArray{T,D} <: AbstractArray{T,D}  # No fixed extents present in the base type

abstract FixedArray1{N,T}       <: FixedArray{T,1}  # length N vector
abstract FixedArray2{N,M,T}     <: FixedArray{T,2}  # NxM matrix
abstract FixedArray3{N,M,P,T}   <: FixedArray{T,3}  # NxMxP tensor
# ... Up to some MaxD dimensional tensors

Doing it this way means you can dispatch on either the dimensionality using FixedArray{T,D}, or the exact D-dimensional shape using FixedArrayD{N1,N2,N3,...,T}.

~~As part of the experiment, I'm seeing what happens if I make FixedArray a subtype of AbstractArray.~~ I ran out of steam on this - another time maybe.

A major overhaul to replace the Tuple size hack in the base FixedArray, which now seems to be broken in 0.5. * Make a set of FixedArray subtypes with fixed dimensionality to replace the Tuple size hack which now seems to be broken in 0.5 * Many changes to core functions to deal with the fallout of the above. * Remove MutableFixedArray for now, since there's no concrete implementations. * New Ar3, Ar4, Ar5 concrete array types

c42f · 2016-07-12T13:46:31Z

@SimonDanisch I think I'm done here. It's a fair bit of churn, but it seems like this approach panned out and will fix the complete breakage in 0.5.

I backed out of making FixedArray a subtype of AbstractArray - there were enough things to fix as it was!

SimonDanisch · 2016-07-12T15:01:15Z

Thanks a lot! This seems reasonable! :)
Where's src/concrete_types.jl at? ;)
By the way, I got more serious about the benchmarksuite: https://github.com/SimonDanisch/FixedArrayBench.jl
I hope that I can integrate it with Julia's benchmarking suite, so that we can more easily track regressions.
We might be able to add it as a travis test PR's need to pass before a merge :)

I backed out of making FixedArray a subtype of AbstractArray

Yeah, lets keep that for later... I tried it a few times and I think as long as we implement it as an abstract interface, we cannot do it unless we improve Base!

c42f · 2016-07-12T21:09:19Z

Where's src/concrete_types.jl at?

Ack too much refactoring the mess I'd made of the git history... things won't work very well without that! Added now.

@andyferris has been prototyping yet another fixed size array package over at https://github.com/andyferris/StaticArrays.jl where he's got the array types as a subtype of AbstractArray. Seems to be working so far.

The names Ar3 and Ar4 seem a bit short for types which may not be widely used. Also add some additional tests and documentation.

andyferris · 2016-07-12T21:51:41Z

Has this broken things? I'm on 0.5 master and FixedSizeArray master and I can't multiply two Mats...

julia> Af*Bf
ERROR: MethodError: no method matching *(::Tuple{Float64,Float64,Float64,Float64}, ::Tuple{Float64,Float64,Float64,Float64})
Closest candidates are:
  *(::Any, ::Any, ::Any, ::Any...)
 in *(::FixedSizeArrays.Mat{4,4,Tuple{Float64,Float64,Float64,Float64}}, ::FixedSizeArrays.Mat{4,4,Tuple{Float64,Float64,Float64,Float64}}) at /home/aferris/.julia/v0.5/FixedSizeArrays/src/ops.jl:293
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

andyferris · 2016-07-12T21:52:50Z

Or maybe the constructor for Mat?

julia> Af = Mat{4,4}(A)
FixedSizeArrays.Mat{4,4,Tuple{Float64,Float64,Float64,Float64}}(
    (0.7397448744879265,0.6693949162818686,0.9311476796229206,0.06709120350902764) (0.7397448744879265,0.6693949162818686,0.9311476796229206,0.06709120350902764) (0.7397448744879265,0.6693949162818686,0.9311476796229206,0.06709120350902764) (0.7397448744879265,0.6693949162818686,0.9311476796229206,0.06709120350902764)
    (0.37538503722900285,0.4419929419024282,0.24653301424257257,0.3648732910609911) (0.37538503722900285,0.4419929419024282,0.24653301424257257,0.3648732910609911) (0.37538503722900285,0.4419929419024282,0.24653301424257257,0.3648732910609911) (0.37538503722900285,0.4419929419024282,0.24653301424257257,0.3648732910609911)
    (0.12111028049933337,0.8437150199258416,0.3838755930341464,0.7975768239181638) (0.12111028049933337,0.8437150199258416,0.3838755930341464,0.7975768239181638) (0.12111028049933337,0.8437150199258416,0.3838755930341464,0.7975768239181638) (0.12111028049933337,0.8437150199258416,0.3838755930341464,0.7975768239181638)
    (0.9308837537576387,0.9532641543205207,0.6819288569516848,0.9280473456548324) (0.9308837537576387,0.9532641543205207,0.6819288569516848,0.9280473456548324) (0.9308837537576387,0.9532641543205207,0.6819288569516848,0.9280473456548324) (0.9308837537576387,0.9532641543205207,0.6819288569516848,0.9280473456548324)
)

c42f · 2016-07-12T21:57:53Z

@andyferris Yes seems broken, though I just checked and current master already has that bug when constructing a Mat from an AbstractArray - I'm not sure it ever worked...

c42f · 2016-07-13T06:58:35Z

Matmul problem is tracked at #145.

c42f · 2016-07-13T07:12:08Z

Uh oh. I've just updated to the latest 0.5, and it seems that someone has just fixed the underlying issue in Base which was causing everything to break with the existing design which uses Tuple as a size parameter.

So now I'm not sure whether to merge this. I think the new design is cleaner in some ways, but having a new set of types for each dimensionality isn't great.

c42f · 2016-07-25T21:25:06Z

In the interests of avoiding API churn, I'm not going to merge this. Bit of a bummer, since it was quite a lot of work! I'll leave the branch around in case we need to bring this back from the dead later.

SimonDanisch · 2016-07-26T12:10:06Z

Sorry to hear! At some point we should figure out how to solve this once and for all and if we want linear tuples ;)

c42f · 2016-07-26T12:41:19Z

TBH I'm wondering whether just migrating to StaticArrays might make more sense once 0.5 comes around - so many of the design decisions in FixedSizeArrays were tradeoffs or workarounds for missing compiler capabilities in 0.4 and I'm a little burned out by trying to work around them ;-) The issue will be avoiding ecosystem fragmentation... at the moment it looks like we have one FSA-equivalent package for each julia version!

SimonDanisch · 2016-07-26T12:48:35Z

yeah sorry, haven't been really helpful lately ;) I'm waiting for a few changes in base and improving GLVisualize is on higher priority right now!
Looks like a nice package for sure! Would need some evaluation, of course ;)

c42f · 2016-07-26T13:04:26Z

No problems at all, I appreciate the massive effort you're putting into GLVisualize. There's only so much time in the day :-)

c42f force-pushed the core-type-rewrite branch from 011f457 to 3c07aa0 Compare July 11, 2016 04:39

c42f force-pushed the core-type-rewrite branch from 3c07aa0 to 6c82c46 Compare July 12, 2016 13:39

c42f changed the title ~~WIP: Core type rewrite~~ Core type rewrite Jul 12, 2016

c42f mentioned this pull request Jul 12, 2016

0.5 splat broken #138

Closed

Add in forgotten concrete_types.jl

12f875f

Rename generic tensor types to FArray{3,4}

d1793b0

The names Ar3 and Ar4 seem a bit short for types which may not be widely used. Also add some additional tests and documentation.

Update documentation

610afbd

Documentation for new abstract types

5aff4de

c42f mentioned this pull request Jul 15, 2016

Nightly struggles with Tuple{X} JuliaLang/julia#17247

Closed

c42f closed this Jul 25, 2016

c42f mentioned this pull request Oct 9, 2016

Relation to FixedSizeArrays.jl JuliaArrays/StaticArrays.jl#11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core type rewrite #144

Core type rewrite #144

c42f commented Jul 11, 2016 •

edited

Loading

c42f commented Jul 12, 2016

SimonDanisch commented Jul 12, 2016

c42f commented Jul 12, 2016

andyferris commented Jul 12, 2016 •

edited

Loading

andyferris commented Jul 12, 2016

c42f commented Jul 12, 2016

c42f commented Jul 13, 2016

c42f commented Jul 13, 2016

c42f commented Jul 25, 2016

SimonDanisch commented Jul 26, 2016

c42f commented Jul 26, 2016

SimonDanisch commented Jul 26, 2016

c42f commented Jul 26, 2016

Core type rewrite #144

Core type rewrite #144

Conversation

c42f commented Jul 11, 2016 • edited Loading

c42f commented Jul 12, 2016

SimonDanisch commented Jul 12, 2016

c42f commented Jul 12, 2016

andyferris commented Jul 12, 2016 • edited Loading

andyferris commented Jul 12, 2016

c42f commented Jul 12, 2016

c42f commented Jul 13, 2016

c42f commented Jul 13, 2016

c42f commented Jul 25, 2016

SimonDanisch commented Jul 26, 2016

c42f commented Jul 26, 2016

SimonDanisch commented Jul 26, 2016

c42f commented Jul 26, 2016

c42f commented Jul 11, 2016 •

edited

Loading

andyferris commented Jul 12, 2016 •

edited

Loading