Mod1 for CartesianIndex

New to Julia
1

Since there is no implementation of mod1 for a CartesianIndex I wanted to implement one myself. However I am confused how to make it performant.

function mod1(CI::CartesianIndex{2}, MOD::CartesianIndex{2})
    CartesianIndex(mod1(CI[1], MOD[1]), mod1(CI[2], MOD[2]))
end

This implementation is performing well is however not very generic.
The more generic version I came up with

function mod1(CI::CartesianIndex{N}, MOD::CartesianIndex{N}) where N
	vals = [mod1(CI[i], MOD[i]) for i in 1:N]
	CartesianIndex(vals...)
end

unfortunately (and obviously) does not perform all too well.
So basically I am wondering how to implement the generic version more performant.

2

Replace this with, using ntuple:

function mod1(CI::CartesianIndex{N}, MOD::CartesianIndex{N}) where N
    vals = ntuple(i -> Base.mod1(CI[i], MOD[i]), N)
    CartesianIndex(vals...)
end
1 Like
3

Normally I would have implemented this by converting to Tuple and broadcasting like

function Base.mod1(CI::CartesianIndex{N}, MOD::CartesianIndex{N}) where N
	CartesianIndex(mod1.(Tuple(CI), Tuple(MOD)))
end

but for some reason this results in allocations so benchmarks poorly relative to the ntuple implementation. Oh well, ntuple it is…

I wasn’t going to post just to say that. What I really wrote in to say is that this function is an example of type piracy and should be avoided except as a pull request to Julia itself. Otherwise, consider naming it something else to avoid the potential for (not so?) hilarious bugs in the future.

4

This seems like something we could support directly. We have mod(::Int, ::UnitRange); the direct corollary here is mod(::CartesianIndex, ::CartesianIndices).

julia> mod(9, 1:7)
2

julia> mod(CartesianIndex((2,9)), CartesianIndices((1:2, 1:7)))
ERROR: MethodError: no method matching mod(::CartesianIndex{2}, ::CartesianIndices{2, Tuple{UnitRange{Int64}, UnitRange{Int64}}})
Stacktrace:
 [1] top-level scope
   @ REPL[5]:1
2 Likes
5

I was thinking about that. The annoying thing is that we don’t have support for (nor an obvious answer for) mod(::Int, ::StepRange) so couldn’t support all CartesianIndices. But it still seems like a nice generalization in the AbstractUnitRange cases where it would apply. It would probably be accepted as a PR to Base without too much fuss.

6

Yeah, the most powerful idiom this would enable is mod(i, CartesianIndices(A)), where you can be sure that it’s a CartesianIndices{<:AbstractUnitRange}. I think it’s fine to leave the other cases as a method error.

7

aside from type piracy

function mod1(CI::CartesianIndex{N}, MOD::CartesianIndex{N}) where N
	vals = (mod1(CI[i], MOD[i]) for i in 1:N)
	CartesianIndex(vals...)
end