Safely using @inbounds

[ali-ramadhan]

Another thing I noticed is that @inbounds is used in places where it can't really be guaranteed, like inside difference operator functions. I couldn't figure out how to test this, but I wonder if @inbounds propagates to all of the functions that are called inside its scope (is that the right language)?

Specifically I'm referring to patterns like the RHS calculation for u, which is @inbounds, but subsequently calls operators like this that are also have @inbounds.

In other words, we only need one @inbounds statement; we don't need multiple nested @inbounds statements. @vchuravy can you confirm/deny this?

A separate issue with @inbounds in places where it can't be guaranteed is that if users write functions / analysis tools using those operators, they might screw up and give out-of-bounds indices to functions that have guaranteed @inbounds (and in consequence get mysterious segfaults?) It seems the natural place for @inbounds is in the scope of a loop.

Originally posted by @glwagner in #163 (comment)

[ali-ramadhan]

@glwagner I created a new issue using your comment as it wasn't very relevant to the original issue.

I agree that it's not guaranteed to be in bounds by itself. I guess we guarantee it by looping over the correct indices but this isn't the cleanest way.

I think you're right that this belongs at the loop level where we can enclose the loop inside an @inbounds block and @propagate_inbounds to inlined functions.

I think @peterahrens suggested using @propagate_inbounds a while back (#58), and @vchuravy (#13 (comment)) just suggested this as well.

Would definitely be good to refactor the code to do this.

Also see: https://docs.julialang.org/en/v1/devdocs/boundscheck/index.html#Propagating-inbounds-1

[glwagner]

Ok, it is relevant to the use of the Field abstraction though; part of my comment was referring to the fact that we always index into f.data rather than f.

[ali-ramadhan]

Ah I see, sorry for misunderstanding. I guess I saw three separate issues:

1. Having to always index f.data instead of f (getindex and setindex! much slower than just accessing array contents directly. #13).
2. Propagating @inbounds (Use @propagate_inbounds #58) and using @inbounds safely (Safely using @inbounds #164).
3. GPU-compatible Field abstractions (isbits Fields and FieldSets #163).

Perhaps all three could be addressed in a single PR (but they could be addressed separately).

[glwagner]

Found this in the docs:

https://docs.julialang.org/en/latest/devdocs/boundscheck/#Propagating-inbounds-1

I think we want to use this notation for our kernel functions, rather than forcing @inbounds.

[vchuravy]

Also see JuliaArrays/StaticArrays.jl#564 (comment)

[glwagner]

We want both @inline and @propagate_inbounds on our kernels --- right?

[vchuravy]

@propagate_inbounds implies @inline and GPUifyLoops will force inline everything anyway on the GPU. So we only need inline annotations for CPU performance

[ali-ramadhan]

Don't think this is an issue anymore, thanks @vchuravy!