end is unsafe with user-defined getindex

[mlubin]

Ref jump-dev/JuMP.jl#730, it seems like there's no way to change the behavior of end translating to size when used within a multidimensional array, which means that it's unsafe to use end to index into JuMP arrays which don't use 1-based indexing. Any hope of a solution to this?

CC @joehuchette @IainNZ
(Thanks @yuyichao for the pointer)

[stevengj]

Shouldn't it call endof instead?

[mlubin]

@stevengj that's the case only for one-dimensional arrays IIRC

[stevengj]

It would be natural to extend it to call endof(A, i) in the i-th dimension.

[mlubin]

I can play around with a PR for that if it has a chance of going through

[stevengj]

Here is the relevant code in julia-syntax.scm.

Replacing size(A,i) with endof(A,i) looks pretty easy, and makes sense to me.

A complication is that it calls trailingsize for the last index in cases with 2 or more indices, so that you can do e.g. A[i,end] even if A is 3-dimensional and get A[i,end,end]. (@StefanKarpinski, why do we allow this?) But I guess you could overload Base.trailingsize in JuMP too.

[mlubin]

trailingsize isn't an issue for us because we already error on getindex when the number of dimensions doesn't match.

[StefanKarpinski]

A complication is that it calls trailingsize for the last index in cases with 2 or more indices, so that you can do e.g. A[i,end] even if A is 3-dimensional and get A[i,end,end]. (@StefanKarpinski, why do we allow this?) But I guess you could overload Base.trailingsize in JuMP too.

Isnt' this "generalized linear indexing", on the chopping block in 0.5, see #14770.

[stevengj]

@mlubin, you would still have to overload it because otherwise it will return size(A,i) for the last dimension. Better yet, we can call trailingendof instead of trailingsize, defined as:

trailingendof(A,n) = n == ndims(A) ? endof(A,n) : trailingsize(A,n)

Good to hear that this "feature" is on the chopping block, though; we can then remove trailingendof and just call endof(A,n).

(So far, the only registered packages that use the unexported Base.trailingsize function seem to be DSP.jl, ZipFile.jl, and Zlib.jl.)

[mlubin]

Will it be easier to fix this after #14770?

[stevengj]

@mlubin, no, I think the two can be done independently if you define trailingendof as I suggested above. After general linear indexing is removed, then trailingendof can be removed and replaced with endof.

[mbauman]

How about we just lower to one function name in all cases? This moves the logic into a place where custom types can intercept it… and it decouples the last index from the size. You just need a three-argument function lastindex(A, dim, is_last_index). Maybe that could even get folded into endof.

Cartesian indices still pose a problem: rand(3,2,1)[CartesianIndex((3,2)), end]… but that's a much harder problem that may be just fine left unresolved.

[stevengj]

@mbauman, because the goal is to get rid of trailingendof, this should remain an undocumented internal function that custom types shouldn't touch. Anyone overriding endof should only have to implement the 2-argument version.

[mbauman]

I believe this should be fixed (or be feasible to fix within an offset array package) via #25763.