Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to MixedDofHandler #279

Merged
merged 11 commits into from
Apr 5, 2020
Merged

Changes to MixedDofHandler #279

merged 11 commits into from
Apr 5, 2020

Conversation

lijas
Copy link
Collaborator

@lijas lijas commented Mar 15, 2020

This makes changes to the MixedDofHandler so it will work the same as regular DofHandler for one celltype. This makes it possible to later merge MixedDofHandler/MixedGrid with DofHandler/Grid, which I think is much cleaner than having two versions that basically do the same thing.

I dont think there will any loss in speed becuase of the way dofhandler stores celldofs, cellcoords and collnodes now.

@codecov-io
Copy link

codecov-io commented Mar 15, 2020

Codecov Report

Merging #279 into master will decrease coverage by 1.56%.
The diff coverage is 70.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #279      +/-   ##
==========================================
- Coverage   88.50%   86.93%   -1.57%     
==========================================
  Files          22       21       -1     
  Lines        1897     1937      +40     
==========================================
+ Hits         1679     1684       +5     
- Misses        218      253      +35     
Impacted Files Coverage Δ
src/JuAFEM.jl 100.00% <ø> (ø)
src/iterators.jl 65.30% <50.00%> (-23.34%) ⬇️
src/Dofs/MixedDofHandler.jl 79.68% <52.08%> (-9.86%) ⬇️
src/Grid/grid.jl 85.83% <87.50%> (+0.24%) ⬆️
src/Dofs/ConstraintHandler.jl 90.72% <100.00%> (-0.55%) ⬇️
src/Dofs/DofHandler.jl 84.15% <100.00%> (+1.17%) ⬆️
src/Export/VTK.jl 79.31% <100.00%> (-1.94%) ⬇️

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 60d4db1...93c6f91. Read the comment docs.

# It is now time to loop over all the cells in our grid. We do this by iterating
# over a `CellIterator`. The iterator caches some useful things for us, for example
# the nodal coordinates for the cell, and the local degrees of freedom.
#+
@inbounds for cell in CellIterator(dh)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does CellIterator not work for MixedDofHandler?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at the moment, but could quite easily fix that so it works for both dofhandler and mixeddofhandler.

@lijas
Copy link
Collaborator Author

lijas commented Mar 17, 2020

Updates:

  • Remove MixedGrid
  • Updated CellIterator to work with MixedDofHandler.
  • I also updated DofHandler, even if the plan is to remove it. The reason for this is to be able to make speed comparisons later maybe...
  • Created AbstractCell because i need to be able to dispatch on Bezier-cells which has the same type s as a QuadraticQuadrilateral 😄

Todo:
Make MixedDofHandler work more smoothly with ConstraintHandler

@lijas lijas requested a review from KristofferC March 17, 2020 19:12
src/iterators.jl Outdated
current_cellid::ScalarWrapper{Int}
nodes::Vector{Int}
coords::Vector{Vec{dim,T}}
dh::DofHandler{dim,N,T,M}
cellset::Vector{Int}
dh::Union{DofHandler{dim,C,T}, MixedDofHandler{dim,C,T}} #Futre: remove DofHandler and rename MixedDofHandler->DofHandler
Copy link
Collaborator

@KristofferC KristofferC Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specialize on this instead?

CellIterator{dim, C, T, DH<:AbstractDofHandler}

@@ -1,5 +1,5 @@

mutable struct MixedGrid{dim,C,T<:Real} <: JuAFEM.AbstractGrid
#=mutable struct Grid{dim,C,T<:Real} <: JuAFEM.AbstractGrid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this file

@@ -54,13 +54,14 @@ struct ConstraintHandler{DH<:AbstractDofHandler,T}
free_dofs::Vector{Int}
values::Vector{T}
dofmapping::Dict{Int,Int} # global dof -> index into dofs and values
bcvalues::Vector{BCValues{T}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store bcvalues in ConstraintHandler instead of getting it from DofHandler

@@ -386,68 +393,27 @@ function meandiag(K::AbstractMatrix)
end



# TODO Remove - almost identical to the previous
function _update!(values::Vector{Float64}, f::Function, faces::Set{Tuple{Int,Int}}, field::Symbol, local_face_dofs::Vector{Int}, local_face_dofs_offset::Vector{Int},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this function anduse the normal _update! function for both MixedDofHandler and DofHandler

@lijas
Copy link
Collaborator Author

lijas commented Mar 23, 2020

All tests work for me and I think it is possible to merge now.

@lijas lijas requested a review from fredrikekre March 24, 2020 07:37
@KristofferC
Copy link
Collaborator

Are the changes to the heat equation example still supposed to be in here?

@@ -110,7 +110,7 @@ update!(ch, 0.0);
# We define a function, `doassemble` to do the assembly, which takes our `cellvalues`,
# the sparse matrix and our DofHandler as input arguments. The function returns the
# assembled stiffness matrix, and the force vector.
function doassemble(cellvalues::CellScalarValues{dim}, K::SparseMatrixCSC, dh::DofHandler) where {dim}
function doassemble(cellvalues::CellScalarValues{dim,T}, K::SparseMatrixCSC, dh::JuAFEM.AbstractDofHandler) where {dim,T}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function doassemble(cellvalues::CellScalarValues{dim,T}, K::SparseMatrixCSC, dh::JuAFEM.AbstractDofHandler) where {dim,T}
function doassemble(cellvalues::CellScalarValues{dim}, K::SparseMatrixCSC, dh::JuAFEM.AbstractDofHandler) where {dim}

seems unused

@@ -127,20 +127,28 @@ function doassemble(cellvalues::CellScalarValues{dim}, K::SparseMatrixCSC, dh::D
f = zeros(ndofs(dh))
assembler = start_assemble(K, f)

#celldofs = zeros(Int, ndofs_per_cell(dh))
#cellcoords = zeros(Vec{dim,T}, JuAFEM.nnodes_per_cell(dh))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm?

# For each cell we also need to reinitialize the cached values in `cellvalues`.
#+
reinit!(cellvalues, cell)
reinit!(cellvalues, cellcoords)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't work with celldata here?

# Always remember to reset the element stiffness matrix and
# force vector since we reuse them for all elements.
#+
fill!(Ke, 0)
fill!(fe, 0)

#JuAFEM.cellcoords!(cellcoords, dh, cellid)
#JuAFEM.celldofs!(celldofs, dh, cellid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm?

@@ -166,7 +174,7 @@ function doassemble(cellvalues::CellScalarValues{dim}, K::SparseMatrixCSC, dh::D
# The last step in the element loop is to assemble `Ke` and `fe`
# into the global `K` and `f` with `assemble!`.
#+
assemble!(assembler, celldofs(cell), fe, Ke)
assemble!(assembler, _celldofs, fe, Ke)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can revert the entire heat_equation example

@@ -54,13 +54,14 @@ struct ConstraintHandler{DH<:AbstractDofHandler,T}
free_dofs::Vector{Int}
values::Vector{T}
dofmapping::Dict{Int,Int} # global dof -> index into dofs and values
bcvalues::Vector{BCValues{T}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So each new bc will have it's own bcvalues? Isn't it more a property of the field/interpolation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MixedDofHandler stores bcvalues in the fieldHandler, and DofHandler stores bcvalues itself, so in able to be able to have the same functionality for both Dohandlers, I must save the bcvalues in ConstraintHandler (I think)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is better to store it in Dirichlet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants