Skip to content

Detect accumulation-like indexing#116

Open
mcabbott wants to merge 4 commits into
masterfrom
accum
Open

Detect accumulation-like indexing#116
mcabbott wants to merge 4 commits into
masterfrom
accum

Conversation

@mcabbott

@mcabbott mcabbott commented Jul 4, 2021

Copy link
Copy Markdown
Owner

Motivated by #115, this tries to detect indexing of this pattern:

x = rand(Int8, 10) .+ 0; y = copy(x)
@tullio y[i] = y[i-1] + y[i]

i.e. writing in-place, to an array which appears on the right, with a shifted index. This i is very unsafe, worse than merely causing race conditions if multi-threaded, the expected result y == cumsum(x) depends on it being done sequentially.

It's possible this should be made an error. But without a shift, @tullio y[i] = y[i] + 1 seems like perfectly cromulent index notation for y .+= 1. And with a new array, of course @tullio z[i] := y[i+1] - y[i] is also fine, z == diff(y).

@mcabbott

mcabbott commented Jul 4, 2021

Copy link
Copy Markdown
Owner Author

The second issue in #115 was about fixing indices on the LHS within shifts. Toy example which now works:

A = zeros(Int, 3,4);
for r in 2:4
    @tullio A[$r-1, c] = c + $r^2 
end
A == [c + r^2 for r in 2:4, c in 1:4]

Two maybe surprises are (1) that the index r isn't checked in any way, for r in 5:10 simply writes out of bounds. And (2) LoopVectorization gives the wrong answer:

using LoopVectorization
A = zeros(Int, 3, 4);
for r in 2:4
    @avx for c in 1:4
        A[r - 1, c] = c + r^2
    end
end
A != [c + r^2 for r in 2:4, c in 1:4]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant