Skip to content
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

Provide internal function for faster access of 3D cell->line_index #13924

Merged
merged 1 commit into from Jul 13, 2022

Conversation

kronbichler
Copy link
Member

@kronbichler kronbichler commented Jun 7, 2022

When looking at a profiler, I observed that cell->line_index(l) and cell->line(l) in 3D is pretty expensive. The main reason is that the function

const auto pair =
accessor.reference_cell().standard_line_to_face_and_line_index(i);
const auto quad_index = pair[0];
const auto line_index =
accessor.reference_cell().standard_to_real_face_line(
pair[1], pair[0], face_orientation_raw(accessor, quad_index));
return accessor.quad(quad_index)->line_index(line_index);
involves pretty heavy operations that cannot be fully optimized through by the compiler. In particular, every single index we check needs to go through the process of determining the appropriate face orientation, pick up the index, and translate from the associated quad's line index to the cell's line index. This can be done more efficiently if we query all 12 line indices (in the sense of index within the array of all lines on a triangulation), because then we only need to look up the orientation of 4 faces and only need to go through the process 4 times. By some loop unrolling, the compiler can optimized away several more statements.

The use of course gets a bit more verbose. There are more places that could be changed in the tria.cc file, but I wanted to show three representative cases in this PR and defer other changes to later PRs. The first one is a bad one (but the one actually triggering my investigations), where we look up the lines to set some refinement information: Here, I need to expand the code (and change the indentation, so look at this without whitespace changes) to construct the line iterator from the line index, with which I can finally work. The other two cases are more gentle, all that changes is the fact that I need to allocate an intermediate array (fixed size).

This could potentially be used in a number of places, such as the get_dof_indices function, so I wonder whether we should move the get_line_indices_of_cell function (or any better name we could think of) into TriaAccessor to have it available there.

Note that my current attempt only optimizes the case for hex reference cells, tetrahedra and similar are still using the old code. I am pretty sure this could be optimized as well, but I have no good cases to look at.

@kronbichler
Copy link
Member Author

This PR should wait for #14066, which introduces the function used here in the appropriate place.

@kronbichler
Copy link
Member Author

I now switched to the new function and applied it in a few more places.

@peterrum
Copy link
Member

peterrum commented Jul 3, 2022

@kronbichler You need to fix the merge conflict.

@kronbichler
Copy link
Member Author

Rebased.

@masterleinad masterleinad merged commit 4661125 into dealii:master Jul 13, 2022
@kronbichler kronbichler deleted the faster_access_to_lines branch September 5, 2022 12:09
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants