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
Implement FEFaceEvaluation::evaluate() for ECL/unstructured meshes #13063
Conversation
|
||
for (unsigned int i = 0; i < 3 * n_components * dofs_per_face; | ||
++i) | ||
temp[i][v] = temp[i][v]; // TODO: we need a second buffer? |
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.
Does scratch_data
have enough space to use as buffer her?
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.
3 * n_components * dofs_per_face
is exactly what we assume as memory before we let scratch_data
begin. Since that additional array is long enough, we should have plenty of more space. But I guess this assignment is still to be updated as it is currently a self-assignment.
for (unsigned int v = 0; v < Number::size(); ++v) | ||
{ |
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 guess it would be better to loop over all 2*d
faces here, instead of looping over each lanes!?
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 guess it depends how often we have a particular face number and change in orientation. I would believe that the current choice might be more efficient for moderate amounts of irregularity, as the other alternative needs to mask out some of the lanes (I think of the results in the hanging nodes algorithms).
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.
Looking at this again, I believe I did not state my point correctly: It is likely that it would help to detect the case where all faces have the same number, and then go to the else
branch below, and only run the per-lane code in the other case. Alternatively, we might want to detect how many of the faces actually pop up, and then only run the code for those cases. That would be strictly less work than the loop over all lanes, as we can't have more different faces than lanes. I guess this was what you had in mind?
I can have a closer look once this is rebased. |
5498c9b
to
c3b6a35
Compare
c3b6a35
to
992ebc3
Compare
I have rebased this 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.
Apart from the question of how we want to handle the lane divergence, I think this is good to go.
992ebc3
to
be8f0d4
Compare
@kronbichler I have updated the PR, pushed some fixes and added tests. What about merging like this. |
/rebuild |
be8f0d4
to
9dab148
Compare
Assert((fe_eval.get_dof_access_index() == | ||
MatrixFreeFunctions::DoFInfo::dof_access_cell && | ||
fe_eval.is_interior_face() == false) == false, |
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 negate this statement?
Assert((fe_eval.get_dof_access_index() == | |
MatrixFreeFunctions::DoFInfo::dof_access_cell && | |
fe_eval.is_interior_face() == false) == false, | |
Assert(fe_eval.get_dof_access_index() != | |
MatrixFreeFunctions::DoFInfo::dof_access_cell || | |
fe_eval.is_interior_face() == true, |
depends on #13056