Fix algorithms that assume linear indexing

[timholy]

I wrote an AbstractArray type that throws an error for linear indexing, and in the course of kicking the tires discovered a number of places where we assume linear indexing (but shouldn't). This PR fixes those places I've found so far.

This could probably be backported to 0.4, with one risky exception which I'll highlight below. Please discuss.

For fun: my new AbstractArray type indexes each dimension from -n:n. By overloading eachindex, I was able to get surprisingly good results. From the remaining problems, unquestionably the most important next step is to define eachindex(A, d) with default value 1:size(A,d). I am likely to play around with this in the near future.

[timholy]

This (and its findmin companion) is the breaking change, because for LinearSlow arrays the returned index will be a CartesianIndex. For 0.5 this is probably the way to go. Assuming we don't want to change it for 0.4, a backport-worthy solution would be to increment a counter in the loop and return that instead.

[mbauman]

Heck, sub2ind is fairly fast. iterstate(x::CartesianIndex) = sub2ind(x)? It'll probably still be a net gain for linear slow arrays.

[timholy]

iterstate's main role is to strip out the CartesianRange from the state variable, returning just the CartesianIndex. (It's been long enough that I now forget why we put it in the state variable, but I didn't take the time to remind myself.)

[mbauman]

Ah, right. I had forgotten about the more complicated state, as well. I think it's for special casing 0-dimensional iterations. sub2ind(iterstate()), then, yes?

[timholy]

Looks like Base._sub2ind(i[1].stop.I, i[2].I) works.

[mbauman]

Very interesting. I've been playing with breaking AbstractArrays in a different way, which has shown some of these same issues. I had been meaning to push that simple fix in ==, too...

[mbauman]

+1. LinearSlow hasn't seen all that much of a workout up until now. But it's essential for 0.5.

As far as backporting, it'll take some thought about how/if we can leverage Compat for the most breaking array changes. I suspect we'll want these sorts of things backported for that to work.

[timholy]

I think with the exception of findmin/findmax, all this only affects internal operations.

I like your sub2ind suggestion. How about I add that for now here, too, and we can deal with API breakages in a separate PR. Then this whole thing can be backported.

[tkelman]

Probably post 0.4.0 at this point

[mbauman]

Oh, definitely.

[timholy]

OK, I've made this non-breaking and added some tests. Without this patch, master fails on these tests at 8 independent locations.

The big delay is because I also verified that this applies without issue to release-0.4. I can submit that as a separate PR after merging this one.

Presuming 0.4.0 is coming soon, this is indeed 0.4.1 material.

[timholy]

@JuliaBackports. Since I have this cherry-picked onto release-0.4 locally, would it help if I submitted a PR against that branch?

[tkelman]

Sure, with a prominent [release-0.4] and "don't merge me until after 0.4.0" in the title and description respectively

[tkelman]

moving the backport pending label to the PR #13235