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
Fix mapping fe field similarity #9769
Fix mapping fe field similarity #9769
Conversation
The diff is best watched without whitespace, |
87da40a
to
20de795
Compare
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.
Good detective work!
OK with the minor changes while you're there. Since you're also changing MappingManifold
, do you want to add a similar test for that class as well?
MappingFEField when the underlying mesh had similarities, even though the | ||
deformed cells did not. This would lead to wrong Jacobians and transformed | ||
shape function gradients, among others, when running deal.II without threads. | ||
This is now fixed. |
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 you update that the same fix applies to MappingManifold
(since you also do this in this patch)?
source/fe/mapping_fe_field.cc
Outdated
} | ||
for (unsigned int k = 0; k < data.n_shape_functions; ++k) | ||
{ | ||
unsigned int comp_k = fe.system_to_component_index(k).first; |
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.
unsigned int comp_k = fe.system_to_component_index(k).first; | |
const unsigned int comp_k = fe.system_to_component_index(k).first; |
source/fe/mapping_fe_field.cc
Outdated
for (unsigned int i = 0; i < spacedim; ++i) | ||
for (unsigned int k = 0; k < data.n_shape_functions; ++k) | ||
{ | ||
unsigned int comp_k = fe.system_to_component_index(k).first; |
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.
While you're there:
unsigned int comp_k = fe.system_to_component_index(k).first; | |
const unsigned int comp_k = fe.system_to_component_index(k).first; |
source/fe/mapping_fe_field.cc
Outdated
for (unsigned int j = 0; j < spacedim; ++j) | ||
for (unsigned int k = 0; k < data.n_shape_functions; ++k) | ||
{ | ||
unsigned int comp_k = fe.system_to_component_index(k).first; |
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.
unsigned int comp_k = fe.system_to_component_index(k).first; | |
const unsigned int comp_k = fe.system_to_component_index(k).first; |
To be honest, I don't know a good way of how to construct a proper manifold that would trigger this. Let me make a short test with a spherical manifold in 2d. |
20de795
to
6cc3b12
Compare
It turned out that it was not too complicated to get that test - just take a refined hyper cube and let the edges be deformed differently with |
We had the cell similarity detection in
MappingFEField
enabled (in analogy toMappingQGeneric
), even though we cannot know for an Euler vector whether there really is a similarity. Therefore, we must completely disable this concept in case of aMappingFEField
and similarily also forMappingManifold
.Fixes #9757