RFC: more efficient cat

[Jutho]

This improves the timings for cat reported here #3645 .
On my machine, before:

julia,catnd_small,173.602586,194.997366,182.986296,7.771113
julia,catnd_large,6.416042,24.993765,9.003105,1.886417
julia,catnd_setind_small,28.170726,42.479779,30.574054,2.421873
julia,catnd_setind_large,5.359645,22.705633,7.671697,1.636532

After:

julia,catnd_small,67.873556,75.480315,69.452319,1.831487
julia,catnd_large,6.451015,22.293655,8.706348,1.590441
julia,catnd_setind_small,27.973874,42.378856,29.205074,1.911954
julia,catnd_setind_large,5.096821,22.897034,7.514883,1.684347

This introduces a mutating version cat! and uses a stagedfunction to generate efficient code for multidimensional arrays, but indirectly, so that the function accepting the varargs argument is a normal (not staged) function. CC: @ViralBShah , @timholy , @simonster

This also reverts the generalization introduced in #7600, where it was allowed to cat along several dimensions. After #7600 got merged, @JeffBezanson didn't seem to happy about that particular generalization. cc @pao

[timholy]

Looks great to me. Sweet implementation.

[simonster]

Yes, this looks good.

[tkelman]

So this will remove the dense blkdiag (though under a different name) functionality?

[Jutho]

@tkelman , I guess I should have added a RFC in the title in the first place. I was hoping to get some discussion started on 2 points:

1. what to do with the generalized version, remove it, keep it, or have it under a dedicated name (e.g. together with blkdiag)?
2. do we want to export the mutating version cat!?

[tkelman]

mutating cat! is a multidimensional generalization of append!, right?

[pao]

It would be nice if dense blkdiag ends up being called blkdiag. That's a win for discoverability, and dense/sparse generality.

[tkelman]

It could either be an alias if the generalized cat functionality stays in, or we could keep just dense blkdiag as a special case (or make a new implementation unrelated to the code here). I suspect that may be the most-used version of generalized concatenation, but I could be wrong.

[ViralBShah]

I think exporting the mutating version of cat! is a good idea.

[Jutho]

I agree. But note , @tkelman , that it is not like append!. cat! doesn't change the size of its first argument. The first argument should just be a preallocated array that will hold the result of the cat operation, much like the copy! or the A_mul_B! type operations.

[Jutho]

Regarding point 1). I have come to agree with @JeffBezanson that the generalised version of cat in #7600 might not be fully appropriate. What sets it apart from the other cat functionality is that it needs to fill part of the output with zeros, so unlike the other cat functions it only makes sense for Number-like element types. I will think about the best way to revive the generalized version under a different interface/name (possible blkdiag).

[timholy]

Agreed we want to export cat!, and the more restricted implementation seems fine to me.

[Jutho]

Ok, I’ll try to finalize this PR tonight.

On 04 Feb 2015, at 11:09, Tim Holy notifications@github.com wrote:

Agreed we want to export cat!, and the more restricted implementation seems fine to me.

—
Reply to this email directly or view it on GitHub #10037 (comment).

[Jutho]

Superseded by #10155 .