Improve get_coordinates! (#536)

* Change getcoordinates for better performance on mixed grids

* make getcoordinates compatible with AbstractGrid interface

* remove duplicate of coordinate query, move cellcoords! to grid.jl

docs/src/manual/grid.md

@@ -168,26 +168,13 @@ Next, we define some helper functions that take care of the node handling.
 Ferrite.getnodes(grid::SmallGrid) = grid.nodes_test
 Ferrite.getnodes(grid::SmallGrid, v::Union{Int, Vector{Int}}) = grid.nodes_test[v]
 Ferrite.getnnodes(grid::SmallGrid) = length(grid.nodes_test)
+Ferrite.get_coordinate_eltype(::SmallGrid) = Float64
 Ferrite.nnodes_per_cell(grid::SmallGrid, i::Int=1) = Ferrite.nnodes(grid.cells_test[i])
 Ferrite.n_faces_per_cell(grid::SmallGrid) = nfaces(eltype(grid.cells_test))
 ```
 
-Finally, we define `getcoordinates`, which is an important function, if we want to assemble a problem.
-The transformation from the reference space to the physical one requires information about the coordinates in order to construct the
-Jacobian. The return of this part is later handled over to `reinit!`.
-
-```julia
-function Ferrite.getcoordinates!(x::Vector{Vec{dim,T}}, grid::SmallGrid, cell::Int) where {dim,T}
-    for i in 1:length(x)
-        x[i] = Vec{dim,T}(grid.nodes_test[grid.cells_test[cell].nodes[i]])
-    end
-end
-
-function Ferrite.getcoordinates(grid::SmallGrid{dim}, cell::Int) where dim
-    nodeidx = grid.cells_test[cell].nodes
-    return [Vec{dim,Float64}(grid.nodes_test[i]) for i in nodeidx]::Vector{Vec{dim,Float64}}
-end
-```
+These definitions make many of `Ferrite`s functions work out of the box, e.g. you can now call 
+`getcoordinates(grid, cellid)` on the `SmallGrid`. 
 
 Now, you would be able to assemble the heat equation example over the new custom `SmallGrid` type.
 Note that this particular subtype isn't able to handle boundary entity sets and so, you can't describe boundaries with it.

src/Dofs/DofHandler.jl

@@ -312,16 +312,6 @@ function cellnodes!(global_nodes::Vector{Int}, grid::AbstractGrid{dim}, i::Int)
     return global_nodes
 end
 
-function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid{dim}, i::Int) where {dim,T}
-    nodes = getcells(grid,i).nodes
-    N = length(nodes)
-    @assert length(global_coords) == N
-    for j in 1:N
-        global_coords[j] = getcoordinates(getnodes(grid,nodes[j]))
-    end
-    return global_coords
-end
-
 cellcoords!(global_coords::Vector{<:Vec}, dh::DofHandler, i::Int) = cellcoords!(global_coords, dh.grid, i)
 
 function celldofs(dh::DofHandler, i::Int)

src/Grid/grid.jl

@@ -14,7 +14,7 @@ struct Node{dim,T}
 end
 Node(x::NTuple{dim,T}) where {dim,T} = Node(Vec{dim,T}(x))
 getcoordinates(n::Node) = n.x
-
+get_coordinate_eltype(::Node{dim,T}) where {dim,T} = T
 
 abstract type AbstractCell{dim,N,M} end
 """
@@ -429,6 +429,8 @@ to a Node.
 @inline getnnodes(grid::AbstractGrid) = length(grid.nodes)
 "Returns the number of nodes of the `i`-th cell."
 @inline nnodes_per_cell(grid::AbstractGrid, i::Int=1) = nnodes(grid.cells[i])
+"Return the number type of the nodal coordinates."
+@inline get_coordinate_eltype(grid::AbstractGrid) = get_coordinate_eltype(first(getnodes(grid)))
 
 """
     getcellset(grid::AbstractGrid, setname::String)
@@ -665,27 +667,40 @@ end
 
 """
     getcoordinates!(x::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::Int)
-Fills the vector `x` with the coordinates of a cell, defined by its cell id.
+    getcoordinates!(x::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::AbstractCell)
+
+Fills the vector `x` with the coordinates of a cell defined by either its cellid or the cell object itself.
 """
-@inline function getcoordinates!(x::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::Int) where {dim,T}
-    #@assert length(x) == N
+
+@inline function getcoordinates!(x::Vector{Vec{dim,T}}, grid::Ferrite.AbstractGrid, cellid::Int) where {dim,T} 
+    cell = getcells(grid, cellid)
+    getcoordinates!(x, grid, cell)
+end
+
+@inline function getcoordinates!(x::Vector{Vec{dim,T}}, grid::Ferrite.AbstractGrid, cell::Ferrite.AbstractCell) where {dim,T}
     @inbounds for i in 1:length(x)
-        x[i] = grid.nodes[grid.cells[cell].nodes[i]].x
+        x[i] = getcoordinates(getnodes(grid, cell.nodes[i]))
     end
+    return x
 end
+
 @inline getcoordinates!(x::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::CellIndex) where {dim, T} = getcoordinates!(x, grid, cell.idx)
 @inline getcoordinates!(x::Vector{Vec{dim,T}}, grid::AbstractGrid, face::FaceIndex) where {dim, T} = getcoordinates!(x, grid, face.idx[1])
 
+# TODO: Deprecate one of `cellcoords!` and `getcoordinates!`, as they do the same thing
+cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid{dim}, i::Int) where {dim,T} = getcoordinates!(global_coords, grid, i) 
+
 """
     getcoordinates(grid::AbstractGrid, cell)
 Return a vector with the coordinates of the vertices of cell number `cell`.
 """
 @inline function getcoordinates(grid::AbstractGrid, cell::Int)
-    # TODO pretty ugly, worth it?
-    dim = typeof(grid.cells[cell]).parameters[1]
-    T = typeof(grid).parameters[3]
-    nodeidx = grid.cells[cell].nodes
-    return [grid.nodes[i].x for i in nodeidx]::Vector{Vec{dim,T}}
+    dim = getdim(grid)
+    T = get_coordinate_eltype(grid)
+    _cell = getcells(grid, cell)
+    N = nnodes(_cell)
+    x = Vector{Vec{dim, T}}(undef, N)
+    getcoordinates!(x, grid, _cell)
 end
 @inline getcoordinates(grid::AbstractGrid, cell::CellIndex) = getcoordinates(grid, cell.idx)
 @inline getcoordinates(grid::AbstractGrid, face::FaceIndex) = getcoordinates(grid, face.idx[1])

test/test_abstractgrid.jl

@@ -15,17 +15,9 @@
     Ferrite.getnodes(grid::SmallGrid) = grid.nodes_test
     Ferrite.getnodes(grid::SmallGrid, v::Union{Int, Vector{Int}}) = grid.nodes_test[v]
     Ferrite.getnnodes(grid::SmallGrid) = length(grid.nodes_test)
+    Ferrite.get_coordinate_eltype(::SmallGrid) = Float64
     Ferrite.nnodes_per_cell(grid::SmallGrid, i::Int=1) = Ferrite.nnodes(grid.cells_test[i])
     Ferrite.n_faces_per_cell(grid::SmallGrid) = nfaces(eltype(grid.cells_test))
-    function Ferrite.getcoordinates!(x::Vector{Vec{dim,T}}, grid::SmallGrid, cell::Int) where {dim,T}
-        for i in 1:length(x)
-            x[i] = Vec{dim,T}(grid.nodes_test[grid.cells_test[cell].nodes[i]])
-        end
-    end
-    function Ferrite.getcoordinates(grid::SmallGrid{dim}, cell::Int) where dim
-        nodeidx = grid.cells_test[cell].nodes
-        return [Vec{dim,Float64}(grid.nodes_test[i]) for i in nodeidx]::Vector{Vec{dim,Float64}}
-    end
 
     nodes = [(-1.0,-1.0); (0.0,-1.0); (1.0,-1.0); (-1.0,0.0); (0.0,0.0); (1.0,0.0); (-1.0,1.0); (0.0,1.0); (1.0,1.0)]
     cells = (Quadrilateral((1,2,5,4)), Quadrilateral((2,3,6,5)), Quadrilateral((4,5,8,7)), Quadrilateral((5,6,9,8)))