-
Notifications
You must be signed in to change notification settings - Fork 100
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
Physical fes #216
Physical fes #216
Conversation
…ace in physical space
…defined in the physical space... NOT WORKING YET
Codecov Report
@@ Coverage Diff @@
## master #216 +/- ##
=======================================
Coverage 88.93% 88.93%
=======================================
Files 143 143
Lines 8866 8866
=======================================
Hits 7885 7885
Misses 981 981 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Some parts have still to be addressed before merging. The major issues are:
-
The
RefTrait
(which I suggest to rename toRefStyle
to match other traits in the code and in julia itself) has to be introduced at theCellFieldLike
level in order to reduce code duplication at theCellField
andCellBasis
level. -
The
evaluate
method that takes care of theRefTrait
has to be implemented forCellFieldLike
. You have implemented forGenericCellBasis
which is not general enough. -
The
evaluate
method forCellDofBasis
should take aCellFieldLike
in the second argument. -
You have introduced a lot of changes that expose the
RefTrait
in the interpolation-related functions ofSingleFieldFESpace
. These functions should be completely agnostic to the trait. The trait should be handled only at theCellDofBasis
andCellFieldLike
level. -
The code in
src/ReferenceFEs/Dofs.jl
should be completely agnostic to theRefTrait
. Again, this should be handled only at theCellDofBasis
andCellFieldLike
level.
If you want, we can discuss this face-to-face since quite critical parts of the code are involved.
src/FESpaces/CellBases.jl
Outdated
@@ -284,6 +297,9 @@ function operate(op,a,b::CellMatrixField) | |||
operate(op,_a,b) | |||
end | |||
|
|||
RefStyle(::CellMatrixField) = Val{true}() | |||
# @santiagobadia : @fverdugo can you check what we need here??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fverdugo can you change it accordingly
@santiagobadia I have fixed a number of issues and pushed to this branch. Take a look to the changes if you want. Perhaps, before merging it would be a good idea to test the physical dofs for multi-field case. Can you write some test? |
Hi @fverdugo
Please take a look at this pull request.
I have created a
RefTrait
forCellField
andCellBasis
in order to determine whether they are defined in the physical of reference space.As a result, I have created some additional internal functions for the physical space case.
I have created a
FunctionField
that given a function creates aField
object, that I have needed in the interpolation subroutines at the physical space.I have fixed many typos I have bumped into, done some limited renaming, etc.
I have added tests for all the previous machinery, being able to solve problems (e.g., using Lagrangian or Raviart-Thomas) defined in the physical space.
There are a couple of thinks that can be done: 1) create new
Kernel
s for performance, e.g., the inverse of the matrix for change of basis, 2) theto_physical
andto_ref
functions but I think it is not that important.Take a look, make comments, etc.