trixi-framework / trixi-framework/Trixi.jl
Consistent use of `@views` or `view(...)`
- Dominant language
- Julia
- Stars
- 731
- Forks
- 166
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 25
Description
Right now, when using a "view" on an array slice (i.e., a sub-array that does not create a copy but directly operates on the original data), we use both the function-style `view(...)`, e.g.,
```julia
# Interpolate to bottom lower left element
multiply_dimensionwise!(
view(u, :, :, :, :, bottom_lower_left_id), forward_lower, forward_lower, forward_lower,
view(old_u, :, :, :, :, old_element_id), u_tmp1, u_tmp2)
```
([ref](https://github.com/trixi-framework/Trixi.jl/blob/9327b7adba74ec60298636fc22b83604d96fec65/src/solvers/dg/3d/amr.jl#L90-L93)) and the macro-style `@views`, e.g.,
```julia
# Copy old element data to new element container
@views elements.u[:, :, :, :, element_id] .= old_u[:, :, :, :, old_element_id]
```
([ref](https://github.com/trixi-framework/Trixi.jl/blob/9327b7adba74ec60298636fc22b83604d96fec65/src/solvers/dg/3d/amr.jl#L41-L42)). I think for the sake of new (Julia) users (and my own sanity, but really it's about the first one), it would be beneficial to use only one of the two approaches in general.
Personally, I prefer `@views` since it is (usually) only a performance enhancement that does not change the code logic, thus I like that it can be "slapped on" an existing expression without changing the expression itself. Also, my brain is much faster in deciphering what `@views u[:, :, :, :, element_id]` means than for `view(u, :, :, :, :, element_id)` (especially in expressions with multiple views), since it intuitively recognizes the brackets `[` `]` as array accessors. However, a case can also be made for `view(...)`, as it moves the "verb" closer to the "subject", i.e., it is immediately clear which arrays are affected.
Before #145 was merged, we had about 2 `view(...)`s and 25 `@views` in `master`; now it is about 60 `view(...)`s and 25 `@views`.
I'm sorry for bringing up yet another consistency issue, but this one can be resolved fairly easy I think (especially, if we just use `@views` everywhere :smiling_imp:)
Contributor guide
Assessment
This issue has not been assessed yet.