Skip to content

Commit 6b431ca

Browse files
authored
Best practice on properly supporting OffsetArrays (#281)
1 parent c52822e commit 6b431ca

File tree

1 file changed

+47
-0
lines changed

1 file changed

+47
-0
lines changed

README.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,49 @@ julia> Origin(AO)(D)
132132

133133
Here, `Origin(AO)` is able to automatically infer and use the indices of `AO`.
134134

135+
## Best practice on adopting OffsetArrays
136+
137+
For some applications, OffsetArrays give users an easy-to-understand interface. However, handling
138+
the non-conventional axes of OffsetArrays requires extra care. Otherwise, the code might
139+
error, crash, or return incorrect results. You can read [the Julialang documentation on
140+
offset](https://docs.julialang.org/en/v1/devdocs/offset-arrays/) for more information. Here
141+
we briefly summarize some of the best practices for users and package authors.
142+
143+
### There is no need to support OffsetArrays for every function
144+
145+
You don't need to support offset arrays for _internal functions_ that only consume standard 1-based
146+
arrays -- it doesn't change or improve anything.
147+
148+
You don't need to support offset arrays for functions that _have no well-defined behavior on custom
149+
axes_. For instance, many linear algebra functions such as matrix multiplication `A * B` does not
150+
have an agreed behavior for offset arrays. In this case, it is a better practice to let users do the
151+
conversion.
152+
153+
The helper function `Base.require_one_based_indexing` can be used to early check the axes and throw
154+
a meaningful error. If your interface functions do not intend to support offset arrays, we recommend
155+
you add this check before starting the real computation.
156+
157+
### use `axes` instead of `size`/`length`
158+
159+
Many implementations assume the array axes start at 1 by writing loops such as `for i in
160+
1:length(x)` or `for i in 1:size(x, 1)`. A better practice is to use `for i in eachindex(x)` or `for
161+
i in axes(x, 1)` -- `axes` provides more information than `size` with no performance overhead.
162+
163+
Also, if you know what indices type you want to use, [`LinearIndices`][doc_LinearIndices] and
164+
[`CartesianIndices`][doc_CartesianIndices] allow you to loop multidimensional arrays efficiently
165+
without worrying about the axes.
166+
167+
### test against OffsetArrays
168+
169+
For package authors that declare support for `AbstractArray`, we recommend having a few test cases
170+
against `OffsetArray` to ensure the function works well for arrays with custom axes. This gives you
171+
more confidence that users don't run into strange situations.
172+
173+
For package users that want to use offset arrays, many numerical correctness issues come from the
174+
fact that `@inbounds` is used inappropriately with the 1-based indexing assumption. Thus for debug
175+
purposes, it is not a bad idea to start Julia with `--check-bounds=yes`, which turns all `@inbounds`
176+
into a no-op and uncover potential out-of-bound errors.
177+
135178
<!-- badges -->
136179

137180
[pkgeval-img]: https://juliaci.github.io/NanosoldierReports/pkgeval_badges/O/OffsetArrays.svg
@@ -147,3 +190,7 @@ Here, `Origin(AO)` is able to automatically infer and use the indices of `AO`.
147190
[docs-stable-url]: https://juliaarrays.github.io/OffsetArrays.jl/stable/
148191
[docs-dev-img]: https://img.shields.io/badge/docs-dev-blue.svg
149192
[docs-dev-url]: https://juliaarrays.github.io/OffsetArrays.jl/dev/
193+
194+
195+
[doc_LinearIndices]: https://docs.julialang.org/en/v1/base/arrays/#Base.LinearIndices
196+
[doc_CartesianIndices]: https://docs.julialang.org/en/v1/base/arrays/#Base.IteratorsMD.CartesianIndices

0 commit comments

Comments
 (0)