trixi-framework / trixi-framework/Trixi.jl

Consistent use of `@views` or `view(...)`

Open
#170 3 comments 0 reactions 0 assignees View on GitHub
discussion good first issue refactoring
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.