-
Notifications
You must be signed in to change notification settings - Fork 99
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
Moment based reffes #1048
base: master
Are you sure you want to change the base?
Moment based reffes #1048
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1048 +/- ##
==========================================
- Coverage 89.04% 85.62% -3.42%
==========================================
Files 197 211 +14
Lines 23887 24336 +449
==========================================
- Hits 21270 20838 -432
- Misses 2617 3498 +881 ☔ View full report in Codecov by Sentry. |
Pullbacks
- homogenized names and updated exported names - updated news.md - handled deprecated APIs - added some docstrings - updated tests with new APIs
…o moment-based-reffes
in place De Casteljau combine De Casteljau's for _derivatives_1d add test
Moment based reffes: refactoring and extension of Gridap.Polynomials
@@ -197,6 +196,7 @@ export bdm | |||
export nedelec | |||
export bezier | |||
export modalC0 | |||
export cr |
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.
shouldn't we name Crouziex-Raviart reffename: crouziex_raviart = CrouziexRaviart() for clarity and consistency with raviart_thomas = RaviartThomas() ?
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.
In the literature, people just refer to it as "cr" just like "bdm". I would suggest that we should name "raviart_thomas" as "RT" instead, which is how people refer to it in the literature and probably modify "cr" to "CR" and "bdm" to "BDM".
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.
I prefer longer names. CR, RT, ... could mean anything. If you start abbreviating you end up with fortran77 codes...
If a user doesn't know that CR means Crouziex-Raviart, maybe they should not be using it.
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.
Also: If you don't know what Raviart-Thomas is, you can google it. Going the other direction is not as easy.
So we've been having a look at the current implementation of Raviart-Thomas and Nedelec. It has become clear that the current implementation
Moreover, there has been interest lately in implementing more types of moment-based discretisations like Crouzeix-Raviart.
We believe the best way to solve all these issues at once is to create some extendable machinery to easily create moment-based FEs.
TODOs:
New RefFEs:
Other: Polynomial bases
The solution to the first issue requires extending Jacobi-type polynomial bases. Although it could be done blindly, I think we could try to merge a lot of code together. In the future we might want to have even more basis of polynomials, which would require a lot of copied code.