DofHandler with any <: AbstractGrid #379

koehlerson · 2021-10-06T09:02:35Z

This PR is in the same spirit of #309 and not such a crowded catastrophe as #307

I added cellnodes! and cellcoords! dispatches for AbstractGrid that only call the interface function of <:AbstractGrid. I noticed that this is needed when we did the first <:AbstractGrid prototype. I'm not sure if these definitions should better live in grid.jl. They assume a concrete homogeneous cells vector right now, since they are only called by DofHandler.

I'd like to do a follow up PR such that a CellIterator also can handle any <:AbstractGrid. Then, we would only need consistent usage of the interface functions and everything should work ootb for properly subtyped grids

codecov-commenter · 2021-10-06T09:10:20Z

Codecov Report

Merging #379 (5f5d25b) into master (9d40af5) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #379   +/-   ##
=======================================
  Coverage   91.50%   91.50%           
=======================================
  Files          22       22           
  Lines        3259     3259           
=======================================
  Hits         2982     2982           
  Misses        277      277

Impacted Files	Coverage Δ
src/Dofs/DofHandler.jl	`83.51% <100.00%> (ø)`
src/iterators.jl	`96.92% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d40af5...5f5d25b. Read the comment docs.

koehlerson · 2021-10-09T14:33:21Z

koehlerson · 2021-10-21T21:34:20Z

koehlerson · 2022-01-07T08:29:14Z

Comparing heat equation on master vs this branch on my laptop (subject to a plethora of fluctuations) in order to see if there is a significant performance drawback. Note this is not intended as performance benchmark but rather as a rough estimate if something was utterly needed w.r.t. type instability

PR

200 x 200 elements
julia> @time include("docs/src/examples/heat_equation.jl")
  1.148711 seconds (187.30 k allocations: 113.092 MiB, 2.27% gc time, 19.11% compilation time)
 
500 x 500 elements
julia> @time include("docs/src/examples/heat_equation.jl")
  6.478253 seconds (400.90 k allocations: 702.862 MiB, 4.93% gc time, 3.64% compilation time)

Master

200 x 200 elements
julia> @time include("docs/src/examples/heat_equation.jl")
 1.152864 seconds (201.76 k allocations: 113.971 MiB, 11.94% gc time, 21.89% compilation time)
 
500 x 500 elements
julia> @time include("docs/src/examples/heat_equation.jl")
 6.244453 seconds (401.77 k allocations: 702.985 MiB, 3.77% gc time, 3.58% compilation time)

koehlerson · 2022-03-06T18:12:07Z

ping @fredrikekre

fredrikekre

Rebase?

fredrikekre · 2022-04-21T15:47:43Z

src/Dofs/DofHandler.jl

+
+function cellnodes!(global_nodes::Vector{Int}, grid::AbstractGrid{dim}, i::Int) where {dim}
+    C = getcelltype(grid)
+    @assert length(global_nodes) == nnodes(C)
+    for j in 1:nnodes(C)
+        global_nodes[j] = getcells(grid, i).nodes[j] ##TODO getnodes(::AbstractCell); only vertices(::AbstractCell) available
+    end 
+    return global_nodes
+end
+
+function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid{dim}, i::Int) where {dim,T}
+    C = getcelltype(grid)
+    @assert length(global_coords) == nnodes(C)
+    global_coords .= getcoordinates(grid, i)
+    return global_coords
+end
+


Is this still needed after the change in https://github.com/Ferrite-FEM/Ferrite.jl/pull/425/files#diff-8db5ed06971ce1004169862f3e499f55666dd630868843be8a53d30ea385e505?

what we really need is:

Ferrite.jl/src/Dofs/DofHandler.jl

Lines 315 to 329 in 81440fa

# shared implementation for all grids

function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::C) where {dim,C<:Ferrite.AbstractCell,T}

nodes = cell.nodes

N = length(nodes)

@assert length(global_coords) == N

for j in 1:N

global_coords[j] = grid.nodes[nodes[j]].x

end

return global_coords

end

function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid, i) where {dim,T}

cell = getcells(grid, i)

cellcoords!(global_coords, grid, cell)

end

from Kim's #445

It's supposed to handle the information for the reinit! which needs the actual coords for any grid. The functionality can be recovered by using the already defined functions on AbstractGrid, so getcoordinates etc.

I can either delete the functions completely and wait for Kim's PR or we take the ones from Kim's PR with the slight improvement suggestion I made over there

We almost have this since #425 though (see Fredriks comment above). Why not just make the changes to that one? Allowing AbstractGrid in the function signature and using getcells + getnodes should literally have no impact on using the function with the regular Grid.

Sorry, I think I misunderstood the comment in the beginning. I changed in the latest commit the functions to work for any <: AbstractGrid (I hope you meant this @kimauth, otherwise I'll revert it) I had to use the getcoordinates dispatch for the Node type as an abstraction in between, otherwise grids always need to use Node as their node type. In the test example I only used a NTuple{dim}, but we can also just assume that Node is always used, if you prefer this.

Looks like what I meant.
I think assuming that a node always has a field x for the coordinate is not too much of a stretch - but considering that we have getcoordinates(node) it's good to use I guess.

…o mk/dofhandler

koehlerson added 2 commits October 6, 2021 10:45

DofHandler with any <:AbstractGrid

1c57c6a

cellnodes, cellcoords! fallback dispatches

1f3795d

koehlerson requested a review from fredrikekre October 6, 2021 09:03

koehlerson added 3 commits October 11, 2021 23:24

added test for subtyping <:AbstractGrid and using it with DofHandler

b04728d

relax isapprox in abstractgrid test

d1f62cd

change isapprox variable names

49ce28b

kimauth approved these changes Jan 7, 2022

View reviewed changes

fredrikekre reviewed Apr 21, 2022

View reviewed changes

koehlerson force-pushed the mk/dofhandler branch from 8dec9b7 to 49ce28b Compare April 22, 2022 06:05

koehlerson added 9 commits April 22, 2022 08:11

DofHandler with any <:AbstractGrid

7d0caab

cellnodes, cellcoords! fallback dispatches

41b77de

added test for subtyping <:AbstractGrid and using it with DofHandler

ce17bdd

relax isapprox in abstractgrid test

5654f66

change isapprox variable names

c28ba14

Merge branch 'mk/dofhandler' of github.com:Ferrite-FEM/Ferrite.jl int…

2357071

…o mk/dofhandler

update cellcoords! and cellnodes

340193c

typo

f1f8a80

add working, general version of cellcoords

5f5d25b

fredrikekre merged commit b46853e into master Apr 22, 2022

fredrikekre deleted the mk/dofhandler branch April 22, 2022 13:33

koehlerson mentioned this pull request Mar 30, 2023

Refactor to use grid and dofhandler interface internally #655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DofHandler with any <: AbstractGrid #379

DofHandler with any <: AbstractGrid #379

koehlerson commented Oct 6, 2021

codecov-commenter commented Oct 6, 2021 •

edited

Loading

koehlerson commented Oct 9, 2021

koehlerson commented Oct 21, 2021

koehlerson commented Jan 7, 2022 •

edited

Loading

koehlerson commented Mar 6, 2022

fredrikekre left a comment

fredrikekre Apr 21, 2022 •

edited

Loading

koehlerson Apr 22, 2022

koehlerson Apr 22, 2022

kimauth Apr 22, 2022

koehlerson Apr 22, 2022

kimauth Apr 22, 2022

	# shared implementation for all grids
	function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid, cell::C) where {dim,C<:Ferrite.AbstractCell,T}
	nodes = cell.nodes
	N = length(nodes)
	@assert length(global_coords) == N
	for j in 1:N
	global_coords[j] = grid.nodes[nodes[j]].x
	end
	return global_coords
	end

	function cellcoords!(global_coords::Vector{Vec{dim,T}}, grid::AbstractGrid, i) where {dim,T}
	cell = getcells(grid, i)
	cellcoords!(global_coords, grid, cell)
	end

DofHandler with any <: AbstractGrid #379

DofHandler with any <: AbstractGrid #379

Conversation

koehlerson commented Oct 6, 2021

codecov-commenter commented Oct 6, 2021 • edited Loading

Codecov Report

koehlerson commented Oct 9, 2021

koehlerson commented Oct 21, 2021

koehlerson commented Jan 7, 2022 • edited Loading

PR

Master

koehlerson commented Mar 6, 2022

fredrikekre left a comment

Choose a reason for hiding this comment

fredrikekre Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

koehlerson Apr 22, 2022

Choose a reason for hiding this comment

koehlerson Apr 22, 2022

Choose a reason for hiding this comment

kimauth Apr 22, 2022

Choose a reason for hiding this comment

koehlerson Apr 22, 2022

Choose a reason for hiding this comment

kimauth Apr 22, 2022

Choose a reason for hiding this comment

codecov-commenter commented Oct 6, 2021 •

edited

Loading

koehlerson commented Jan 7, 2022 •

edited

Loading

fredrikekre Apr 21, 2022 •

edited

Loading