Wrote additional high level API assembly functions #652
Conversation
Codecov Report
@@ Coverage Diff @@
## master #652 +/- ##
==========================================
+ Coverage 87.70% 88.03% +0.33%
==========================================
Files 134 134
Lines 14315 14679 +364
==========================================
+ Hits 12555 12923 +368
+ Misses 1760 1756 -4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hi @amartinhuertas thanks for the PR!
See my minor comments below.
src/FESpaces/Assemblers.jl
Outdated
| @@ -270,44 +270,88 @@ function assemble_matrix(f::Function,a::Assembler,U::FESpace,V::FESpace) | |||
| assemble_matrix(a,collect_cell_matrix(U,V,f(u,v))) | |||
| end | |||
|
|
|||
| function assemble_matrix!(A,f::Function,a::Assembler,U::FESpace,V::FESpace) | |||
There was a problem hiding this comment.
Start the argument list with ::Function objects so that we can use do-block syntax. Also type-annotate the new array arguments, I.e.
assemble_matrix!(f::Function,A::AbstractMatrix,a::Assembler,U::FESpace,V::FESpace)
assemble_matrix_and_vector!(f::Function,b::Function,M::AbstractMatrix,r::AbstractVector,a::Assembler,U::FESpace,V::FESpace)Apply this minor changes to all new introduced assembly functions.
There was a problem hiding this comment.
Start the argument list with ::Function objects so that we can use do-block syntax
I though that the parameters that are modified in a !-like function should appear first in the list of parameters by convention. From your comment, I guess this is not strictly necessary, right?
…dditional_highlevel_assembly_functions
* Started the argument list with ::Function objects so that we can use do-block syntax.
src/FESpaces/Assemblers.jl
Outdated
| function assemble_matrix(f,a::Assembler,U::FESpace,V::FESpace) | ||
| assemble_matrix(a,collect_cell_matrix(U,V,f)) | ||
| end | ||
|
|
||
| function assemble_matrix!(A::AbstractMatrix,f,a::Assembler,U::FESpace,V::FESpace) |
There was a problem hiding this comment.
f is not a function, isnt?
src/FESpaces/Assemblers.jl
Outdated
| function assemble_vector(f,a::Assembler,V::FESpace) | ||
| assemble_vector(a,collect_cell_vector(V,f)) | ||
| end | ||
|
|
||
| function assemble_vector!(b::AbstractVector,f,a::Assembler,V::FESpace) |
There was a problem hiding this comment.
f is not a function, isnt?
There was a problem hiding this comment.
f is a callable object, usually a Function.
to be consistent we have 2 main options:
- add missing ::Function everywhere in this file, where we accept a function
- remove all ::Function anotations to allow generic callable objects
2 is more general, but 1 is less disruptive wrt what we have now and also works since the weak form is always defined via functions.
There was a problem hiding this comment.
I still dont understand why f it is function/callable object in this particular function. See, e.g.,
function collect_cell_vector(test::FESpace,a::DomainContribution)
w = []
r = []
for trian in get_domains(a)
cell_vec = get_contribution(a,trian)
@assert ndims(eltype(cell_vec)) == 1
cell_vec_r = attach_constraints_rows(test,cell_vec,trian)
rows = get_cell_dof_ids(test,trian)
push!(w,compress_contributions(cell_vec_r,trian))
push!(r,compress_ids(rows,trian))
end
(w,r)
end
a is a callable object?
There was a problem hiding this comment.
You are right. It is not a function. it is an object for which collect_cell_vector and related functions are defined.
There was a problem hiding this comment.
ok, in any case I had to put the f and b args first in all function signatures to let all tests pass ...
src/FESpaces/Assemblers.jl
Outdated
| function assemble_matrix_and_vector(f,b,a::Assembler,U::FESpace,V::FESpace) | ||
| assemble_matrix_and_vector(a,collect_cell_matrix_and_vector(U,V,f,b)) | ||
| end | ||
|
|
||
| function assemble_matrix_and_vector!(M,r,f,b,a::Assembler,U::FESpace,V::FESpace) |
There was a problem hiding this comment.
and here (and type-annotate array inputs)
There was a problem hiding this comment.
oops ... I forgot this one
There was a problem hiding this comment.
f is not a function, isnt?
src/FESpaces/Assemblers.jl
Outdated
| function assemble_matrix(f,U::FESpace,V::FESpace) | ||
| a = SparseMatrixAssembler(U,V) | ||
| assemble_matrix(f,a,U,V) | ||
| end | ||
|
|
||
| function assemble_matrix!(A,f,U::FESpace,V::FESpace) |
src/FESpaces/Assemblers.jl
Outdated
| function assemble_vector(f,V::FESpace) | ||
| a = SparseMatrixAssembler(V,V) | ||
| assemble_vector(f,a,V) | ||
| end | ||
|
|
||
| function assemble_vector!(b,f,V::FESpace) |
src/FESpaces/Assemblers.jl
Outdated
| function assemble_matrix_and_vector(f,b,U::FESpace,V::FESpace) | ||
| a = SparseMatrixAssembler(U,V) | ||
| assemble_matrix_and_vector(f,b,a,U,V) | ||
| end | ||
|
|
||
| function assemble_matrix_and_vector!(M,r,f,b,U::FESpace,V::FESpace) |
|
Hi @fverdugo ... I already addressed all your comments. Anyway there is still something I do not understand. Some times we pass arguments to the |
|
You are right. It is not a function. it is an object for which collect_cell_vector and related functions are defined. E.g. a DomainContribution |
fverdugo
deleted the
developed_additional_highlevel_assembly_functions
branch
No description provided.