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

Speed up DoFAccessorImplementation::get_dof_indices #14066

Merged
merged 3 commits into from Jun 29, 2022

Conversation

kronbichler
Copy link
Member

@kronbichler kronbichler commented Jun 28, 2022

Part of #2000.

This PR currently builds on #14063, I will rebase once that PR is reviewed and merged. Edit: rebased

This PR extracts the fast access to line indices initially written in #13924 and puts it to the appropriate place in internal::TriaAccessorImplementation::Implementation. I also added a similar function for the line orientations. With these changes, I can get the indices of all lines with twice the instruction count it took to get a single index/orientation previously - in 3D this is a 6x improvement. This is the first new commit on top of #14063.

The second commit then applies the new function to the extraction of dof_indices in DoFAccessorImplementation. With this commit, we're further closing the gap to the cell_dof_indices_cache, and in fact, once this PR is merged, we should be able to delete the cache because the performance is now good enough.

There is one final step I still see on the performance side, the case when the faces/lines are not in standard orientation. We then call FiniteElement::adjust_quad_dof_index_for_face_orientation, which extracts bits for the orientation/rotation/flip, only to then again put them together in a similar way. We should create an overload of that function in a follow-up PR that directly picks up the raw face orientation that we store nowadays.

Comment on lines +1242 to +1245
// For other shapes (tetrahedra, wedges, pyramids), we do not
// currently implement an optimized function
for (unsigned int l = 0; l < std::min(12U, cell.n_lines()); ++l)
line_orientations[l] = cell.line_orientation(l);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will be forever slow :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not really difficult to apply a similar scheme also for the other shapes. Essentially, one needs to unroll the decision in

{
static const std::array<unsigned int, 2> table[6] = {
{{0, 0}}, {{0, 1}}, {{0, 2}}, {{1, 1}}, {{1, 2}}, {{2, 1}}};
return table[line];
}
else if (*this == ReferenceCells::Pyramid)
{
static const std::array<unsigned int, 2> table[8] = {{{0, 0}},
{{0, 1}},
{{0, 2}},
{{0, 3}},
{{1, 2}},
{{2, 1}},
{{1, 1}},
{{2, 2}}};
return table[line];
}
else if (*this == ReferenceCells::Wedge)
{
static const std::array<unsigned int, 2> table[9] = {{{0, 0}},
{{0, 2}},
{{0, 1}},
{{1, 0}},
{{1, 1}},
{{1, 2}},
{{2, 0}},
{{2, 1}},
{{3, 1}}};
return table[line];
to the code here, so go through three or four faces and pick the lines there. All the rest will be similar to the hexahedral case. In case we have good tests, this should be easy to try out and get running (< 1 hour of work).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we tackle this as part of #14068?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we tackle this as part of #14068?

If we see a significant slow down, yes.

Comment on lines -1236 to +1242
accessor.get_fe(fe_index).template n_dofs_per_object<0>();
dof_handler.get_fe(0).template n_dofs_per_object<0>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened with fe_index?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. This is the MG path. Wouldn't it make sense to add an assert that fe_index is ineed 0 or default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I added the check in line 1344 below, not here, as I do not want to pick up the index here again.

@drwells drwells merged commit 96aa719 into dealii:master Jun 29, 2022
@kronbichler kronbichler deleted the speed_up_access_dof_indices branch September 5, 2022 12:09
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
…indices

Speed up DoFAccessorImplementation::get_dof_indices
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