-
Notifications
You must be signed in to change notification settings - Fork 21
Mimetic Spectral Elements #29
base: master
Are you sure you want to change the base?
Conversation
Okay, so this pull request is not very useful here. FIAT and UFL pull requests should be filed against upstream to be merged: https://bitbucket.org/fenics-project/fiat If you want to receive code review from the Firedrake team before filing the pull request upstream, then please file the pull request against |
Rebased onto master now that this is where things are happening. |
Conflicts: FIAT/__init__.py FIAT/quadrature.py
Updated this with master. |
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.
Well, the comments above explain why this has not been review thus far...
# GNU Lesser General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU Lesser General Public License | ||
# along with FIAT. If not, see <http://www.gnu.org/licenses/>. |
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.
We are only insert SPDX header lines since #33.
return symbas | ||
|
||
|
||
class _EdgeElement(FiniteElement): |
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.
Why are all these names above starting with an underscore? Are they meant to be module private?
My personal opinion is further development and refactoring may make you want to use function in other module, even though those functions were (originally) intended to be private. Such private -> public change then either introduces inconsistency (if the name is not changed), or introduces unnecessary name change which pollutes the diff at review.
Most of the time one just imports what one needs anyway, and in those cases when from package.module import *
is meaningful, one can set the list of exported names through the __all__
variable.
|
||
|
||
class EdgeGaussLobattoLegendre(_EdgeElement): | ||
def __init__(self, ref_el, degree): |
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.
Some docstring would be nice.
|
||
|
||
class EdgeExtendedGaussLegendre(_EdgeElement): | ||
def __init__(self, ref_el, degree): |
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.
docstring missing
# GNU Lesser General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU Lesser General Public License | ||
# along with FIAT. If not, see <http://www.gnu.org/licenses/>. |
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.
SPDX header line
|
||
if __name__ == '__main__': | ||
import os | ||
pytest.main(os.path.abspath(__file__)) |
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.
We normally don't add these anymore at the bottom of test files.
@@ -0,0 +1,60 @@ | |||
# Copyright (C) 2019 Imperial College London and others |
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.
dubious copyright line
# GNU Lesser General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU Lesser General Public License | ||
# along with FIAT. If not, see <http://www.gnu.org/licenses/>. |
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.
SPDX header line
@pytest.mark.parametrize("degree", list(range(2, 7))) | ||
def test_egl_basis_values(degree): | ||
"""Ensure that integrating a simple monomial produces the expected results.""" | ||
from FIAT import ufc_simplex, ExtendedGaussLegendre, make_quadrature |
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.
Avoid local imports unless there is a good reason.
|
||
if __name__ == '__main__': | ||
import os | ||
pytest.main(os.path.abspath(__file__)) |
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.
cargo-cult at the bottom of test file
This pull request adds mimetic spectral elements (described at https://www.sciencedirect.com/science/article/pii/S0021999113006414 and https://arxiv.org/abs/1111.4304), both the primal and dual complexes. This is done by adding the relevant 1D H1 element for the dual complex: Extended Gauss Legendre (EGL); and the L2 edge elements associated with GLL and EGL. These are non-Ciarlet finite elements that use a basis that histopolates (http://people.math.sfu.ca/~nrobidou/public_html/prints/histogram/histogram.pdf) rather than interpolates. The nD deRham complex on hypercubes is then accessible using the variant keyword (either “mse” or “dualmse”) with the relevant elements (DQ, Q, RTCF, etc.).