Apply suggestions from code review

Co-authored-by: Hendrik Ranocha <[email protected]>

Author: vchuravy

docs/src/heterogeneous.md

@@ -82,7 +82,7 @@ CuArray
 
 ## Element-type conversion with `Trixi.trixi_adapt`.
 
-We can use [`Trixi.trixi_adapt`](@ref) to perform both an element-type and a storage-type adoption
+We can use [`Trixi.trixi_adapt`](@ref) to perform both an element-type and a storage-type adoption:
 
 ```jldoctest adapt
 julia> C = Container(zeros(3))
@@ -98,7 +98,8 @@ Container{CuArray{Float32, 1, CUDA.DeviceMemory}}(Float32[0.0, 0.0, 0.0])
 ```
 
 !!! note
-    `adapt(Array{Float32}, C)` is tempting but will do the wrong thing in the presence of `StaticArrays`.
+    `adapt(Array{Float32}, C)` is tempting, but it will do the wrong thing
+    in the presence of `SVector`s and similar arrays from StaticArrays.jl.
 
 
 ## Writing GPU kernels
@@ -123,10 +124,10 @@ end
     `element`, pass all required fields as arguments, but make sure to `@unpack` them from
     their structs in advance.
 
-2.  Where `trixi_rhs_fct` is called, get the backend, i.e. the hardware we are currently
+2.  Where `trixi_rhs_fct` is called, get the backend, i.e., the hardware we are currently
     running on via `trixi_backend(x)`.
-    This will, e.g., work with `u_ode`. Internally, `KernelAbstractions.jl`'s `get_backend`
-    will be called, i.e. `KernelAbstractions.jl` has to know the type of `x`.
+    This will, e.g., work with `u_ode`. Internally, KernelAbstractions.jl's `get_backend`
+    will be called, i.e., KernelAbstractions.jl has to know the type of `x`.
 
     ```julia
     backend = trixi_backend(u_ode)
@@ -143,8 +144,8 @@ end
     end
     ```
 
-4.  When `backend` is a `Backend` (a type defined by `KernelAbstractions.jl`), write a
-    `KernelAbstractions.jl` kernel:
+4.  When `backend` is a `Backend` (a type defined by KernelAbstractions.jl), write a
+    KernelAbstractions.jl kernel:
     ```julia
     function trixi_rhs_fct(backend::Backend, mesh, equations, solver, cache, args)
         nelements(solver, cache) == 0 && return nothing  # return early when there are no elements

examples/p4est_2d_dgsem/elixir_advection_basic_gpu.jl

@@ -49,6 +49,7 @@ stepsize_callback = StepsizeCallback(cfl = 1.6)
 
 # Create a CallbackSet to collect all callbacks such that they can be passed to the ODE solver
 callbacks = CallbackSet(summary_callback, stepsize_callback)
+# TODO: GPU. The `analysis_callback` needs to be updated for GPU support
 # analysis_callback, save_solution, stepsize_callback)
 
 ###############################################################################

src/auxiliary/containers.jl

@@ -366,7 +366,7 @@ function trixi_backend(x)
     return get_backend(x)
 end
 
-# TODO: Remove once we have https://github.com/SciML/RecursiveArrayTools.jl/pull/455
+# TODO: After https://github.com/SciML/RecursiveArrayTools.jl/pull/455 we need to investigate the right way to handle StaticArray as uEltype for MultiDG.
 function trixi_backend(x::VectorOfArray)
     u = parent(x)
     # FIXME(vchuravy): This is a workaround because KA.get_backend is ambivalent of where a SArray is residing.

src/solvers/dgsem_p4est/containers_parallel.jl

@@ -128,7 +128,7 @@ mutable struct P4estMPIMortarContainer{NDIMS, uEltype <: Real, RealT <: Real, ND
     u::uArray                                      # [small/large side, variable, position, i, j, mortar]
     local_neighbor_ids::Vector{Vector{Int}}        # [mortar][ids]
     local_neighbor_positions::Vector{Vector{Int}}  # [mortar][positions]
-    node_indices::Matrix{NTuple{NDIMS, Symbol}} # [small/large, mortar]
+    node_indices::Matrix{NTuple{NDIMS, Symbol}}    # [small/large, mortar]
     normal_directions::Array{RealT, NDIMSP2}       # [dimension, i, j, position, mortar]
     # internal `resize!`able storage
     _u::uVector
@@ -222,6 +222,7 @@ function init_mpi_mortars!(mpi_mortars, mesh::ParallelP4estMesh, basis, elements
 end
 
 function Adapt.adapt_structure(to, mpi_mortars::P4estMPIMortarContainer)
+    # TODO: GPU
     # Only parts of this container are adapted, since we currently don't
     # use `local_neighbor_ids`, `local_neighbor_positions`, `normal_directions`
     # on the GPU. If we do need them we need to redesign this to use the VecOfArrays

test/test_amdgpu.jl

@@ -16,8 +16,8 @@ EXAMPLES_DIR = joinpath(examples_dir(), "p4est_2d_dgsem")
                         # Expected errors are exactly the same as with TreeMesh!
                         l2=8.311947673061856e-6,
                         linf=6.627000273229378e-5,)
-    # # Ensure that we do not have excessive memory allocations
-    # # (e.g., from type instabilities)
+    # Ensure that we do not have excessive memory allocations
+    # (e.g., from type instabilities)
     let
         t = sol.t[end]
         u_ode = sol.u[end]
@@ -44,12 +44,12 @@ end
     using AMDGPU
     @test_trixi_include(joinpath(EXAMPLES_DIR, "elixir_advection_basic_gpu.jl"),
                         # Expected errors are exactly the same as with TreeMesh!
-                        l2=nothing,   # [Float32(8.311947673061856e-6)],
-                        linf=nothing, # [Float32(6.627000273229378e-5)],
+                        l2=nothing,   # TODO: GPU. [Float32(8.311947673061856e-6)],
+                        linf=nothing, # TODO: GPU. [Float32(6.627000273229378e-5)],
                         RealT=Float32,
                         real_type=Float32,
                         storage_type=ROCArray,
-                        sol=nothing,) # TODO: Remove this once we can run the simulation on the GPU 
+                        sol=nothing,) # TODO: GPU. Remove this once we can run the simulation on the GPU 
     # # Ensure that we do not have excessive memory allocations
     # # (e.g., from type instabilities)
     # let

test/test_cuda.jl

@@ -44,12 +44,12 @@ end
     using CUDA
     @test_trixi_include(joinpath(EXAMPLES_DIR, "elixir_advection_basic_gpu.jl"),
                         # Expected errors are exactly the same as with TreeMesh!
-                        l2=nothing,   # [Float32(8.311947673061856e-6)],
-                        linf=nothing, # [Float32(6.627000273229378e-5)],
+                        l2=nothing,   # TODO: GPU. [Float32(8.311947673061856e-6)],
+                        linf=nothing, # TODO: GPU. [Float32(6.627000273229378e-5)],
                         RealT=Float32,
                         real_type=Float32,
                         storage_type=CuArray,
-                        sol=nothing,) # TODO: Remove this once we can run the simulation on the GPU
+                        sol=nothing,) # TODO: GPU. Remove this once we can run the simulation on the GPU
     # # Ensure that we do not have excessive memory allocations
     # # (e.g., from type instabilities)
     # let