-
Notifications
You must be signed in to change notification settings - Fork 93
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
WIP: New vector interpolations and mappings #798
base: master
Are you sure you want to change the base?
Conversation
… error though (test fails)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #798 +/- ##
==========================================
- Coverage 93.57% 89.40% -4.18%
==========================================
Files 39 42 +3
Lines 6073 6739 +666
==========================================
+ Hits 5683 6025 +342
- Misses 390 714 +324 ☔ View full report in Codecov by Sentry. |
src/FEValues/EdgeValues.jl
Outdated
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.
Can we put this feature (+ tests) in a separate PR?
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.
Yes, it will.
Sorry that didn't update much on the plan with this PR, but it will need to be split into several sub-PRs. However, it is easier to work with the full one, and it can also serve as a reference when reviewing the sub-PRs later.
@@ -49,3 +49,24 @@ dofs defined on a specific entity. Hence, not overloading of the dof functions w | |||
element with zero dofs. Also, it should always be double checked that everything is consistent as | |||
specified in the docstring of the corresponding function, as inconsistent implementations can | |||
lead to bugs which are really difficult to track down. | |||
|
|||
## Vector interpolation properties | |||
### Hdiv interpolations |
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.
### Hdiv interpolations | |
### H(div) interpolations |
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.
It might make sense to add some comments about the De-Rahm complex here for readers not familiar with the topic.
@@ -9,9 +9,9 @@ solution for a Maxwell problem, when vectorized `Lagrange` interpolations conver | |||
|
|||
![Results for different discretizations](maxwell.png) | |||
|
|||
**Figure 1**: The results of this tutorial, showing how the analytical solution is not found when discretizing | |||
**Figure 1**: The results of this tutorial, showing how the exact solution is not found when discretizing |
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.
Stylistic detail, but maybe we can get rid of the negation here. Maybe something like:
**Figure 1**: The results of this tutorial, showing how the exact solution is not found when discretizing | |
**Figure 1**: The results of this tutorial, showing how the approximation converges to the wrong solution when discretizing |
@@ -219,28 +213,28 @@ function create_data(tr::Triangulation, fieldname::Symbol, a; f = identity) | |||
return data | |||
end | |||
|
|||
# ## Analytical implementation | |||
# ## Exact implementation |
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.
Stylistic again, but exact implementation
does sound off.
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.
Yes, but it would be good to highlight that this refers to a chosen solution, not an analytical solution to an existing problem. But I'm not used to working with such "reverse" problems, so perhaps that is still conventionally called "analytical"?
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.
And btw, I'll open a separate PR with the tutorials in a while.
Working on separating out the functionality changes. Just keeping this branch including all just to avoid missing anything...
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.
You mean manufactured solution
?
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 think it's different, because it is one solution that fulfills the pde, exactly so no source terms are added. The influence comes from the boundary terms...
In https://doi.org/10.1016/j.compstruc.2019.106175 they mention also "Method of exact solutions" but couldn't find any ref or explanation to this...
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.
Ohh, I see. I found an explanation for this term here https://onlinelibrary.wiley.com/doi/pdf/10.1002/fld.660 in 3.1. So essentially we are looking for analytical solutions for corner cases of the PDE.
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.
Thanks for that paper that defines it. The statement,
In the method of exact solutions, specialized cases are identied where exact solutions
exist to a given set of governing equations. For the Euler and Navier–Stokes sets of
equations, there are only a limited number of exact solutions. Furthermore, these
exact solutions may not exercise all of the terms in the governing equations. For
two infnite parallel plates, one moving relative to the other (Couette flow), the
velocity profile is linear, hence the diffusion term, a second derivative of velocity,
is identically zero and therefore would not be fully exercised.
I guess somewhat in that direction, but here we only (as I understood the reference page) choose some field fulfilling the pde without a source term, but the boundary conditions are found by evaluating the resulting reaction loads/boundary constraints. Hence we don't find an exact/analytical solution to a simplified problem.
But since it is an exact fulfillment of the PDE, but not of the problem, I think exact makes sense. However, the header is still not good, probably better to have "Implementation of the exact solution" (or just "Exact solution").
Implements Hcurl (Nedelec) and Hdiv (RaviartThomas and BrezziDouglasMarini) interpolations and the associated mappings.
To ensure that the infrastructure works well, I would aim to have the following here
Finalization plan
The diff is too large for effective review, so this PR should not be merged, but I will split it into several smaller PRs.